-
Notifications
You must be signed in to change notification settings - Fork 0
[P-835] [P-898] [P-950]: Sdk event queue, prevent duplicate events within 1 minute, sends visitor id to authorizer #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…thin 1 minute, sends visitor id to authorizer
P-835 SDK: Refactor with event queue & batching
Currently we send and retry events individually. We should instead queue and batch the sending of events. Allow batching of events. This is needed to scale because Tinybird's event API has a 1000 RPS limit https://www.tinybird.co/docs/get-data-in/ingest-apis/events-api#limits References
P-950 Some users with only 1 Session are labelled as 'Returning'
If you go to https://app.formo.so/teams/94f54b87-0209-4411-90dc-0e24a186c729/projects/formo.so/users Screenshot 2025-02-11 at 17.46.25.png And click on some of the 'Returning' users (which should have Screenshot 2025-02-11 at 17.47.53.png This user only has 1 session (Feb 7 2025) P-898 Debug why a high number of duplicate events are emitted
Screenshot 2025-02-17 at 17.52.03.png Could this be an issue in the private registerAddressChangedListener(): void {
const listener = (...args: unknown[]) =>
this.onAddressChanged(args[0] as string[]);
this._provider?.on("accountsChanged", listener);
this._providerListeners["accountsChanged"] = listener;
const onAddressDisconnected = this.onAddressDisconnected.bind(this);
this._provider?.on("disconnect", onAddressDisconnected);
this._providerListeners["disconnect"] = onAddressDisconnected;
} |
| "access": "public" | ||
| }, | ||
| "license": "MIT", | ||
| "dependencies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the bundle size now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
98kb now
| } | ||
| this.eventQueue.enqueue(requestData, (err, _, data) => { | ||
| if (err) { | ||
| console.error(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw later on there should be a config for 'log levels'
Unless the SDK enables logs, we shouldn't print logs by default since it's only for our debugging
Tracked here https://linear.app/getformo/issue/P-1022/add-log-levels-to-sdk-config
|
The beforeunload handler is async, but the browser might not wait for it to complete |
|
Improving duplicate detection suggestions
|
|
@TamHuynhTee Can you also rename the |
|
@TamHuynhTee LGTM, feel free to publish Also please review https://github.com/getformo/formono/pull/335 |
No description provided.