regression: skip "ResizeObserver loop" warning in Bugsnag reports#40722
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughThe PR modifies error boundary initialization to filter benign ResizeObserver loop warnings from Bugsnag error reporting. An ChangesBugsnag ResizeObserver error filtering
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/meteor/client/views/root/OutermostErrorBoundary.tsx`:
- Around line 26-27: Remove the inline implementation comment about the
"ResizeObserver loop" from the OutermostErrorBoundary component and keep the
existing filter logic intact (e.g., the error-check that skips the benign
ResizeObserver message inside OutermostErrorBoundary/componentDidCatch or the
error filter function). Delete the comment block so the TSX follows the project
guideline while leaving the self-descriptive filter code (the conditional that
detects "ResizeObserver loop" or the related error-filtering function)
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b9c1f51-d857-4531-a4f6-a4c32f022824
📒 Files selected for processing (1)
apps/meteor/client/views/root/OutermostErrorBoundary.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: 📦 Build Packages
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/root/OutermostErrorBoundary.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/root/OutermostErrorBoundary.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/views/root/OutermostErrorBoundary.tsx
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-8.5.0 #40722 +/- ##
================================================
Coverage ? 69.91%
================================================
Files ? 3327
Lines ? 126575
Branches ? 22013
================================================
Hits ? 88498
Misses ? 34791
Partials ? 3286
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Just for documentation sake, this PR is related to this issue |
Proposed changes (including videos or screenshots)
While scrolling a channel, Bugsnag was being flooded with
errorevents whose message is:This is not a real error - it is a benign, informational browser signal (per the W3C Resize Observer spec) dispatched as an
ErrorEventfor historical reasons, which makes error trackers misclassify it as an unhandled exception. It became visible in 8.5.0 because PR #40105 virtualized the message list using virtua, whose per-itemResizeObservers fire in bursts during scroll and flood Bugsnag. (What the warning means and how it manifests is detailed in Further comments.)This PR adds an
onErrorcallback toBugsnag.start()that drops events whose top error message starts withResizeObserver loop. The warning still occurs in the browser (developers can still see it in the console); we only stop reporting a benign browser signal as a crash. Maintainers of comparable virtualization libraries recommend the same: in TanStack/virtual#531 - a different library than virtua, but the same per-itemResizeObserverpattern and the same warning - a collaborator recommends ignoring/filtering this exact message rather than trying to fix it (details in Further comments).String matching is unavoidable: the event has no usable stack (
lineNumber: 0) andErrorEvent.errorisnull, so the message string is the only identifier the browser exposes.startsWith('ResizeObserver loop')also covers Safari's older variant,ResizeObserver loop limit exceeded.This is a deliberate, palliative fix. It stops the Bugsnag noise immediately with zero performance cost, but it does not remove the warning itself - the root cause is virtua's per-item
ResizeObservers. We filter for now because the source-level fixes all trade performance or maintenance for it (see Further comments); a proper non-regressing fix stays open for later.Issue(s)
CORE-2209
Steps to test or reproduce
BUGSNAG_CLIENT=00000000000000000000000000000000(a dummy 000 works - requests return 401, which is fine for this test).notify.bugsnag.com.notify.bugsnag.comcarrying theResizeObserver loopmessage (up to Bugsnag's per-sessionmaxEvents: 10cap, after which further events are dropped client-side).notify.bugsnag.comfor theResizeObserver loopmessage during the same scroll. As a control, a genuinely thrownErrorstill POSTs normally - confirming the filter drops only the targeted message.Further comments
What the warning means and how it shows up in our app
ResizeObserverdelivers size-change callbacks at most once per animation frame. The browser runs the observer callbacks, then checks whether those callbacks caused new size changes; if they did and it can't deliver them within the same frame, it defers the rest to the next frame and emits this warning. In plain terms: "a resize callback changed layout, which produced more resizes than I could deliver this frame - I postponed the rest." Nothing is lost; the deferred notifications arrive on the next frame, which is why it is benign.In our app it manifests during message-list scrolling, roughly like this:
ResizeObservermeasures each newly mounted row.It is triggered two ways, both real:
dangerouslySetInnerHTML- forces a large re-measurement, the worst case for the loop. These amplify the rate but are not required for it.useGetMoreadds a spike at the top of the list: scrolling up loads older history and prepends a batch of messages at once, causing a burst of simultaneous mounts and measurements in a single frame.Why filter instead of fixing the warning at the source?
We confirmed empirically (instrumenting
window.ResizeObserverand attributing each warning to the observers that fired in the same frame) that the dominant generator is virtua's own internalResizeObservers - not our code. virtua exposes no API to inject a custom/deferred observer; it readsResizeObserveroff the element's window at observe-time.This is also what the maintainers of TanStack recommend. In TanStack/virtual#531, a TanStack Virtual collaborator recommends ignoring/filtering the warning rather than fixing it:
…suggesting the exact message-based filter we use here (
ResizeObserver loop completed with undelivered notifications./ResizeObserver loop limit exceeded), and explicitly noting that "patching it withrequestAnimationFramewill not solve the underlying issue" - which is why we did not pursue the rAF shim below.The related perf PR does not fix this. #40695 replaces
flushSyncwithstartTransitionand wraps ouruseGetMorecallbacks inrequestAnimationFrame. Tested with and without it under an identical scripted scroll, the warning count is unchanged. As established above, the warnings come from virtua's observers, not our hook, so cleaning up our hook cannot move the count.The obvious source-level fix degrades performance. Wrapping
window.ResizeObserverso every callback defers one frame breaks the loop precondition, but adds ~16ms latency to everyResizeObserverin the app (~25 use sites incl. interactive ones like drag and composer auto-grow) to fix what is only a reporting problem. TanStack Virtual exposes exactly this as an optionuseAnimationFrameWithResizeObserver, but its own docs say it "typically should not be enabled" - ~16ms with no performance benefit, and the warning "usually indicates a deeper issue that should be fixed." A true upstream fix in virtua isn't readily available either.Also, inokawa/virtua#470 discuss same thing.
In short: a no-cost stopgap now; a source-level fix stays open for when one exists that doesn't cost
performance.
Follow-up task: CORE-2264
Summary by CodeRabbit