-
Notifications
You must be signed in to change notification settings - Fork 727
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
fix: connection robustness and flaky tests #5515
Conversation
…etconnect-monorepo into fix/single-connection
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.
LGTM, left comments mainly around readability.
} | ||
}; | ||
this.relayer.events.on(RELAYER_EVENTS.publish, onPublish); | ||
const initialPublish = createExpiringPromise( |
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.
Not directly related to this PR but createExpiringPromise
could use Promise.race
to be simplified and avoid potential race conditions.
This approach eliminates the need for manual clearTimeout
calls because once the Promise settles, the other Promise in the race is ignored, and the timer will be garbage collected.
export function createExpiringPromise<T>(
promise: Promise<T>,
expiry: number,
expireErrorMessage?: string,
): Promise<T> {
const timeoutPromise = new Promise<T>((_, reject) =>
setTimeout(() => reject(new Error(expireErrorMessage)), expiry),
);
return Promise.race([promise, timeoutPromise]);
}
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.
will test it out duing tech debt week
Description
Reworked several parts of the client to address general stability issues and to increase robustness of the socket connections
Type of change
How has this been tested?
canary -
2.17.2-canary-rcnt-3
deployment - https://react-dapp-v2-git-chore-connection-test-reown-com.vercel.app/
Checklist
Additional Information (Optional)
Please include any additional information that may be useful for the reviewer.