Skip to content

fix(web-vitals): Add error handling for invalid object keys in WeakMap #18809

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.

Sign up for GitHub

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

Merged
s1gr1d merged 2 commits into from
Jan 14, 2026
Merged

fix(web-vitals): Add error handling for invalid object keys in WeakMap #18809

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ede440d
fix(web-vitals): Add error handling for invalid object keys in `WeakMap`
s1gr1d Jan 14, 2026
56b32b1
Merge branch 'develop' into sig/web-vitals-fix
nicohrubec Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions packages/browser-utils/src/metrics/web-vitals/lib/initUnique.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,16 @@ const instanceMap: WeakMap<object, unknown> = new WeakMap();
* identity object was previously used.
*/
export function initUnique<T>(identityObj: object, ClassObj: new () => T): T {
if (!instanceMap.get(identityObj)) {
instanceMap.set(identityObj, new ClassObj());
try {
if (!instanceMap.get(identityObj)) {
instanceMap.set(identityObj, new ClassObj());
}
return instanceMap.get(identityObj)! as T;
} catch (e) {
// --- START Sentry-custom code (try/catch wrapping) ---
// Fix for cases where identityObj is not a valid key for WeakMap (sometimes a problem in Safari)
Copy link
Member

@JPeer264 JPeer264 Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Is this issue easily reproducible? If so it'd be nice if we can somehow add a E2E test, if not I'm fine as is

Copy link
Member Author

@s1gr1d s1gr1d Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, sadly it's not :/

// Just return a new instance without caching it in instanceMap
return new ClassObj();
Copy link

@cursor cursor bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test for fix PR per review rules

Low Severity · Bugbot Rules

Per the review rules: "When reviewing a fix PR, check if the PR includes at least one unit, integration or e2e test that tests the regression this PR fixes." This fix PR adds error handling for invalid WeakMap keys in Safari but the diff contains no accompanying test to verify the fix works correctly. A test that simulates the Safari TypeError scenario would help prevent regressions.

Fix in Cursor Fix in Web

}
return instanceMap.get(identityObj)! as T;
// --- END Sentry-custom code ---
}
Loading