From 0407cc324ae943153cbc452a96b19239e17bf818 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 25 Sep 2026 04:52:52 +0000 Subject: [PATCH 01/13] Fix RollbarContext without a payload (#102), make onRender work and document it (#88) #102: rollbar.js has no default `payload` option, so RollbarContext and useRollbarContext threw "Cannot read properties of undefined (reading 'context')" unless the config set one. Read it with `?.`. Restoring the unset context on unmount now uses '' instead of undefined, which configure() ignores, so the context used to stay set after unmount. rollbar.js sends '' for an unset context anyway. #88: by default the context is set on mount, after the children have rendered, so errors an ErrorBoundary catches during the first render are reported with the previous context. `onRender` sets it first, but it was undocumented and had bugs: - it called setState during render, which React warns about - it ignored later changes to the `context` prop The previous context is now kept on the instance. componentDidUpdate applies a changed `context` under onRender. The README documents onRender, when to use it and why it isn't the default. Co-Authored-By: Claude Opus 5.5 --- README.md | 31 ++- src/hooks/use-rollbar-context.js | 6 +- src/rollbar-context.js | 48 ++--- src/tests/components/rollbar-context.test.js | 198 +++++++++++++++++++ 4 files changed, 256 insertions(+), 27 deletions(-) create mode 100644 src/tests/components/rollbar-context.test.js diff --git a/README.md b/README.md index 8af8414..1185cbe 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ next month. - [Pass a Fallback UI](#pass-a-fallback-ui) - [`RollbarContext` Component](#rollbarcontext-component) - [Basic Usage](#basic-usage) + - [Setting the context before children render](#setting-the-context-before-children-render) - [Using with React Router](#using-with-react-router) - [Functions](#functions) - [`historyContext` to create `history.listener`](#historycontext-to-create-historylistener) @@ -357,8 +358,9 @@ export function App(props) { Use the `RollbarContext` component to declaratively set the `context` value used by [Rollbar.js] when it's sending any messages to [Rollbar]. -This works for your `ErrorBoundary` from above or any other log or message sent to [Rollbar] while the `RollbarContext` -is mounted on the tree. +The context applies to any log or message sent to [Rollbar] while the `RollbarContext` is mounted on the tree. For +errors that your `ErrorBoundary` from above catches while the children are first rendering, see +[Setting the context before children render](#setting-the-context-before-children-render). Like `ErrorBoundary` above, `RollbarContext` relies on a [`Provider`] for an instance of a [Rollbar.js] client. @@ -376,6 +378,28 @@ function HomePage() { } ``` +#### Setting the context before children render + +By default `RollbarContext` sets the context when it mounts, which React does after the children have rendered and +mounted. An error thrown while they're first rendering, which is what an `ErrorBoundary` usually catches, is +therefore reported with the previous context. + +Add the `onRender` prop to set the context during the first render instead, before the children render: + +```javascript + + + + + +``` + +Either way, a change to the `context` prop is applied, and the previous context is restored on unmount. + +`onRender` sets the context from inside `render`. If React throws that render away without committing it, for +example during a transition that gets interrupted, nothing restores the previous context, so the context can stay +set to a page that never showed. That's why it isn't the default. + #### Using with React Router It's useful to set the `context` in [Rollbar] associated with areas of your application. On the server it's usually @@ -554,6 +578,9 @@ function ContactDetails({ contactId }) { As an alternative to the [`RollbarContext`] component, you can use the `useRollbarContext` hook in your [Functional Component] to set the `context` in the [Rollbar.js] client provided by the [`Provider`] above in the React Tree. +The hook sets the context in an effect, so like `RollbarContext` without `onRender`, it doesn't apply to errors +thrown while the component and its children are first rendering. + Here's an example of using it in several components: ```javascript diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index 46b6a2b..5d3fe0f 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -14,10 +14,12 @@ export function useRollbarContext(ctx = '', isLayout = false) { invariant(typeof ctx === 'string', '`ctx` must be a string'); const rollbar = useRollbar(); (isLayout ? useLayoutEffect : useEffect)(() => { - const origCtx = rollbar.options.payload.context; + const origCtx = rollbar.options.payload?.context; rollbar.configure({ payload: { context: ctx } }); return () => { - rollbar.configure({ payload: { context: origCtx } }); + // configure() ignores undefined values; '' is what rollbar.js sends + // for an unset context anyway. + rollbar.configure({ payload: { context: origCtx ?? '' } }); }; }, [ctx]); } diff --git a/src/rollbar-context.js b/src/rollbar-context.js index df1394c..161aca0 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -17,48 +17,50 @@ export class RollbarContext extends Component { static contextType = Context; - firstRender = true; + // The context in effect before this component set its own, restored on + // unmount. Kept on the instance rather than in state because with onRender + // it is captured during render, where setState isn't allowed. + previousContext = undefined; + contextSet = false; - constructor(props) { - super(props); - this.state = { previousContext: null }; - } - - changeContext = (storePrevious = true) => { + changeContext = () => { const rollbar = getRollbarFromContext(this.context); - const { context } = this.props; - if (storePrevious) { - this.setState({ previousContext: rollbar.options.payload.context }); + if (!this.contextSet) { + // rollbar.js has no default payload, so options.payload is undefined + // unless the config sets it. + this.previousContext = rollbar.options.payload?.context; + this.contextSet = true; } - rollbar.configure({ payload: { context } }); + rollbar.configure({ payload: { context: this.props.context } }); }; componentDidMount() { - const { onRender } = this.props; - if (!onRender) { - this.changeContext(true); + // With onRender the context was already set during the first render. + if (!this.contextSet) { + this.changeContext(); } } - componentDidUpdate() { - const { onRender } = this.props; - if (!onRender) { - this.changeContext(false); + componentDidUpdate(prevProps) { + const { onRender, context } = this.props; + if (!onRender || context !== prevProps.context) { + this.changeContext(); } } componentWillUnmount() { const rollbar = getRollbarFromContext(this.context); - const { previousContext } = this.state; - rollbar.configure({ payload: { context: previousContext } }); + // configure() ignores undefined values, so restoring an unset context + // needs ''. rollbar.js sends '' for an unset context anyway. + rollbar.configure({ payload: { context: this.previousContext ?? '' } }); + this.contextSet = false; } render() { const { onRender } = this.props; - if (onRender && this.firstRender) { - this.changeContext(true); + if (onRender && !this.contextSet) { + this.changeContext(); } - this.firstRender = false; return this.props.children; } } diff --git a/src/tests/components/rollbar-context.test.js b/src/tests/components/rollbar-context.test.js new file mode 100644 index 0000000..84c68de --- /dev/null +++ b/src/tests/components/rollbar-context.test.js @@ -0,0 +1,198 @@ +// Plain JS rather than TSX: `onRender` isn't in index.d.ts's RollbarContext +// props on main (#162 adds it), and ts-jest type-checks TSX tests. +import React from 'react'; +import { render } from '@testing-library/react'; +import Rollbar from 'rollbar'; +import { + ErrorBoundary, + Provider, + RollbarContext, + useRollbarContext, +} from '../rollbar-react'; + +const makeRollbar = (config = {}) => + new Rollbar({ + accessToken: 'POST_CLIENT_ITEM_TOKEN', + enabled: false, + ...config, + }); + +const contextOf = (rollbar) => rollbar.options.payload?.context; + +describe('RollbarContext', () => { + let consoleError; + + beforeEach(() => { + consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleError.mockRestore(); + }); + + // #102 + it('works when the config has no payload', () => { + const rollbar = makeRollbar(); + expect(rollbar.options.payload).toBeUndefined(); + + const { unmount } = render( + + +
+ + , + ); + expect(contextOf(rollbar)).toBe('home'); + + // Not undefined: configure() ignores undefined values, which used to + // leave 'home' in place after unmount. + unmount(); + expect(contextOf(rollbar)).toBe(''); + }); + + it('sets the context on mount and restores the previous one on unmount', () => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + + const { rerender, unmount } = render( + + +
+ + , + ); + expect(contextOf(rollbar)).toBe('home'); + + rerender( + + +
+ + , + ); + expect(contextOf(rollbar)).toBe('about'); + + unmount(); + expect(contextOf(rollbar)).toBe('root'); + }); + + // #88 + describe('when a child throws while rendering', () => { + const renderThrowing = (onRender) => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + const reported = []; + rollbar.error = jest.fn(() => reported.push(contextOf(rollbar))); + const Throw = () => { + throw new Error('render error'); + }; + render( + + + + + + + , + ); + expect(rollbar.error).toHaveBeenCalledTimes(1); + return reported[0]; + }; + + it('reports with the previous context by default', () => { + expect(renderThrowing(false)).toBe('root'); + }); + + it('reports with this context when onRender is set', () => { + expect(renderThrowing(true)).toBe('home'); + }); + }); + + describe('with onRender', () => { + it('sets the context before children render', () => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + let seen; + const Child = () => { + seen = contextOf(rollbar); + return null; + }; + + const { unmount } = render( + + + + + , + ); + expect(seen).toBe('home'); + + unmount(); + expect(contextOf(rollbar)).toBe('root'); + }); + + it('does not call setState during render', () => { + const rollbar = makeRollbar(); + render( + + +
+ + , + ); + expect(consoleError).not.toHaveBeenCalled(); + }); + + it('follows changes to the context prop', () => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + const ui = (context) => ( + + +
+ + + ); + + const { rerender, unmount } = render(ui('home')); + rerender(ui('about')); + expect(contextOf(rollbar)).toBe('about'); + + unmount(); + expect(contextOf(rollbar)).toBe('root'); + }); + + it('keeps the innermost context when nested', () => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + render( + + + +
+ + + , + ); + expect(contextOf(rollbar)).toBe('inner'); + }); + }); +}); + +describe('useRollbarContext', () => { + // #102 + it('works when the config has no payload', () => { + const rollbar = makeRollbar(); + const Page = () => { + useRollbarContext('home'); + return null; + }; + + const { unmount } = render( + + + , + ); + expect(contextOf(rollbar)).toBe('home'); + + // Not undefined: configure() ignores undefined values, which used to + // leave 'home' in place after unmount. + unmount(); + expect(contextOf(rollbar)).toBe(''); + }); +}); From e5456cda72615f54173df19c369653faa4bfcdd8 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 25 Sep 2026 05:01:19 +0000 Subject: [PATCH 02/13] Convert RollbarContext tests to TSX now that #162's typings are in The tests were plain JS only because index.d.ts on main had no `onRender`. Stacked on #162 they type-check as TSX, like the other component tests. Co-Authored-By: Claude Opus 5.5 --- ...ntext.test.js => rollbar-context.test.tsx} | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) rename src/tests/components/{rollbar-context.test.js => rollbar-context.test.tsx} (91%) diff --git a/src/tests/components/rollbar-context.test.js b/src/tests/components/rollbar-context.test.tsx similarity index 91% rename from src/tests/components/rollbar-context.test.js rename to src/tests/components/rollbar-context.test.tsx index 84c68de..b4dd931 100644 --- a/src/tests/components/rollbar-context.test.js +++ b/src/tests/components/rollbar-context.test.tsx @@ -1,5 +1,3 @@ -// Plain JS rather than TSX: `onRender` isn't in index.d.ts's RollbarContext -// props on main (#162 adds it), and ts-jest type-checks TSX tests. import React from 'react'; import { render } from '@testing-library/react'; import Rollbar from 'rollbar'; @@ -10,17 +8,18 @@ import { useRollbarContext, } from '../rollbar-react'; -const makeRollbar = (config = {}) => +const makeRollbar = (config: Rollbar.Configuration = {}) => new Rollbar({ accessToken: 'POST_CLIENT_ITEM_TOKEN', enabled: false, ...config, }); -const contextOf = (rollbar) => rollbar.options.payload?.context; +const contextOf = (rollbar: Rollbar): unknown => + rollbar.options.payload?.context; describe('RollbarContext', () => { - let consoleError; + let consoleError: jest.SpyInstance; beforeEach(() => { consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); @@ -77,10 +76,13 @@ describe('RollbarContext', () => { // #88 describe('when a child throws while rendering', () => { - const renderThrowing = (onRender) => { + const renderThrowing = (onRender: boolean) => { const rollbar = makeRollbar({ payload: { context: 'root' } }); - const reported = []; - rollbar.error = jest.fn(() => reported.push(contextOf(rollbar))); + const reported: unknown[] = []; + rollbar.error = jest.fn(() => { + reported.push(contextOf(rollbar)); + return { uuid: '' }; + }); const Throw = () => { throw new Error('render error'); }; @@ -109,7 +111,7 @@ describe('RollbarContext', () => { describe('with onRender', () => { it('sets the context before children render', () => { const rollbar = makeRollbar({ payload: { context: 'root' } }); - let seen; + let seen: unknown; const Child = () => { seen = contextOf(rollbar); return null; @@ -142,7 +144,7 @@ describe('RollbarContext', () => { it('follows changes to the context prop', () => { const rollbar = makeRollbar({ payload: { context: 'root' } }); - const ui = (context) => ( + const ui = (context: string) => (
From d722ee7680a3b9615e1149e1d9559fc703aa8017 Mon Sep 17 00:00:00 2001 From: Brian Rue Date: Fri, 25 Sep 2026 14:00:28 -0700 Subject: [PATCH 03/13] Keep the innermost context when nested RollbarContexts change Review feedback: with nested onRender contexts, changing the outer `context` prop overwrote the inner one that was still mounted. React runs update lifecycles child-first, so the outer component applied its value last. Nesting was already broken without onRender and in the hook: children mount first, so the outer context won at mount, and unmounting restored in the wrong order, leaving the inner context set. The component and the hook now share a list of active contexts per Rollbar client (src/context-stack.js). Each entry takes an order number on first render, which ranks parents before children, and the innermost active entry is applied on every change. When the list empties, the context from before the first entry is restored. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 3 + src/context-stack.js | 55 ++++++++++++ src/hooks/use-rollbar-context.js | 18 ++-- src/rollbar-context.js | 33 +++---- src/tests/components/rollbar-context.test.tsx | 90 ++++++++++++++++--- 5 files changed, 159 insertions(+), 40 deletions(-) create mode 100644 src/context-stack.js diff --git a/README.md b/README.md index 1185cbe..82326bd 100644 --- a/README.md +++ b/README.md @@ -396,6 +396,9 @@ Add the `onRender` prop to set the context during the first render instead, befo Either way, a change to the `context` prop is applied, and the previous context is restored on unmount. +`RollbarContext` components can be nested, including with the `useRollbarContext` hook. The innermost one that's +mounted sets the context. When it unmounts, the next one out applies again, with its current `context`. + `onRender` sets the context from inside `render`. If React throws that render away without committing it, for example during a transition that gets interrupted, nothing restores the previous context, so the context can stay set to a page that never showed. That's why it isn't the default. diff --git a/src/context-stack.js b/src/context-stack.js new file mode 100644 index 0000000..0d701a1 --- /dev/null +++ b/src/context-stack.js @@ -0,0 +1,55 @@ +// The RollbarContext components and useRollbarContext hooks that are +// currently setting a context, per Rollbar client. The innermost one decides +// the client's context. When the last one goes away, the context from before +// the first one is restored. +const stacks = new WeakMap(); + +let lastOrder = 0; + +// Parents render before their children, so an order number taken during the +// first render ranks nested contexts from outermost to innermost, even though +// React mounts (and runs effects for) children first. A context that mounts +// later under an existing one also ranks after it. +export function nextContextOrder() { + lastOrder += 1; + return lastOrder; +} + +function applyInnermost(rollbar, stack) { + let innermost = null; + for (const entry of stack.entries) { + if (!innermost || entry.order > innermost.order) { + innermost = entry; + } + } + rollbar.configure({ payload: { context: innermost.context } }); +} + +// `entry` is `{ order, context }`, owned by the caller. Adds it, or applies +// its new `context` if it's already there. +export function setContext(rollbar, entry) { + let stack = stacks.get(rollbar); + if (!stack) { + // rollbar.js has no default payload, so options.payload is undefined + // unless the config sets it. + stack = { base: rollbar.options.payload?.context, entries: new Set() }; + stacks.set(rollbar, stack); + } + stack.entries.add(entry); + applyInnermost(rollbar, stack); +} + +export function removeContext(rollbar, entry) { + const stack = stacks.get(rollbar); + if (!stack || !stack.entries.delete(entry)) { + return; + } + if (stack.entries.size) { + applyInnermost(rollbar, stack); + return; + } + stacks.delete(rollbar); + // configure() ignores undefined values, so restoring an unset context + // needs ''. rollbar.js sends '' for an unset context anyway. + rollbar.configure({ payload: { context: stack.base ?? '' } }); +} diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index 5d3fe0f..e5c6f8a 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -1,8 +1,9 @@ 'use client'; import invariant from 'tiny-invariant'; -import { useEffect, useLayoutEffect } from 'react'; +import { useEffect, useLayoutEffect, useState } from 'react'; import { useRollbar } from './use-rollbar'; +import { nextContextOrder, removeContext, setContext } from '../context-stack'; // Simple version does its job // export function useRollbarContext(context) { @@ -13,13 +14,12 @@ import { useRollbar } from './use-rollbar'; export function useRollbarContext(ctx = '', isLayout = false) { invariant(typeof ctx === 'string', '`ctx` must be a string'); const rollbar = useRollbar(); - (isLayout ? useLayoutEffect : useEffect)(() => { - const origCtx = rollbar.options.payload?.context; - rollbar.configure({ payload: { context: ctx } }); - return () => { - // configure() ignores undefined values; '' is what rollbar.js sends - // for an unset context anyway. - rollbar.configure({ payload: { context: origCtx ?? '' } }); - }; + // Where this component sits among nested contexts; see context-stack.js. + const [entry] = useState(() => ({ order: nextContextOrder(), context: ctx })); + const useEffectOfType = isLayout ? useLayoutEffect : useEffect; + useEffectOfType(() => { + entry.context = ctx; + setContext(rollbar, entry); }, [ctx]); + useEffectOfType(() => () => removeContext(rollbar, entry), []); } diff --git a/src/rollbar-context.js b/src/rollbar-context.js index 161aca0..a4c679c 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -3,6 +3,7 @@ import { Component } from 'react'; import PropTypes from 'prop-types'; import { Context, getRollbarFromContext } from './provider'; +import { nextContextOrder, removeContext, setContext } from './context-stack'; export class RollbarContext extends Component { static propTypes = { @@ -17,26 +18,21 @@ export class RollbarContext extends Component { static contextType = Context; - // The context in effect before this component set its own, restored on - // unmount. Kept on the instance rather than in state because with onRender - // it is captured during render, where setState isn't allowed. - previousContext = undefined; - contextSet = false; + // Where this component sits among nested contexts; see context-stack.js. + // Kept on the instance rather than in state because with onRender it is + // added during render, where setState isn't allowed. + entry = { order: nextContextOrder(), context: undefined }; + active = false; changeContext = () => { - const rollbar = getRollbarFromContext(this.context); - if (!this.contextSet) { - // rollbar.js has no default payload, so options.payload is undefined - // unless the config sets it. - this.previousContext = rollbar.options.payload?.context; - this.contextSet = true; - } - rollbar.configure({ payload: { context: this.props.context } }); + this.entry.context = this.props.context; + this.active = true; + setContext(getRollbarFromContext(this.context), this.entry); }; componentDidMount() { // With onRender the context was already set during the first render. - if (!this.contextSet) { + if (!this.active) { this.changeContext(); } } @@ -49,16 +45,13 @@ export class RollbarContext extends Component { } componentWillUnmount() { - const rollbar = getRollbarFromContext(this.context); - // configure() ignores undefined values, so restoring an unset context - // needs ''. rollbar.js sends '' for an unset context anyway. - rollbar.configure({ payload: { context: this.previousContext ?? '' } }); - this.contextSet = false; + removeContext(getRollbarFromContext(this.context), this.entry); + this.active = false; } render() { const { onRender } = this.props; - if (onRender && !this.contextSet) { + if (onRender && !this.active) { this.changeContext(); } return this.props.children; diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index b4dd931..d58dd36 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -159,20 +159,88 @@ describe('RollbarContext', () => { unmount(); expect(contextOf(rollbar)).toBe('root'); }); + }); +}); - it('keeps the innermost context when nested', () => { - const rollbar = makeRollbar({ payload: { context: 'root' } }); - render( - - - +describe('nested contexts', () => { + type NestedProps = { outer: string; inner: string; showInner: boolean }; + + const HookInner = ({ context }: { context: string }) => { + useRollbarContext(context); + return null; + }; + const HookOuter = ({ outer, inner, showInner }: NestedProps) => { + useRollbarContext(outer); + return showInner ? : null; + }; + + const modes: [string, React.ComponentType][] = [ + [ + 'RollbarContext', + ({ outer, inner, showInner }) => ( + + {showInner && ( +
- - , - ); - expect(contextOf(rollbar)).toBe('inner'); - }); + )} + + ), + ], + [ + 'RollbarContext with onRender', + ({ outer, inner, showInner }) => ( + + {showInner && ( + +
+ + )} + + ), + ], + ['useRollbarContext', HookOuter], + [ + 'useRollbarContext inside RollbarContext', + ({ outer, inner, showInner }) => ( + + {showInner && } + + ), + ], + ]; + + it.each(modes)('%s: the innermost context wins', (_, Nested) => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + const ui = (props: NestedProps) => ( + + + + ); + + // React mounts children before their parents. + const { rerender, unmount } = render( + ui({ outer: 'outer', inner: 'inner', showInner: true }), + ); + expect(contextOf(rollbar)).toBe('inner'); + + // The outer context changes while the inner one is still mounted. + rerender(ui({ outer: 'outer2', inner: 'inner', showInner: true })); + expect(contextOf(rollbar)).toBe('inner'); + + rerender(ui({ outer: 'outer2', inner: 'inner2', showInner: true })); + expect(contextOf(rollbar)).toBe('inner2'); + + // The outer context's current value, not the one it had when the inner + // one mounted. + rerender(ui({ outer: 'outer2', inner: 'inner2', showInner: false })); + expect(contextOf(rollbar)).toBe('outer2'); + + rerender(ui({ outer: 'outer2', inner: 'inner3', showInner: true })); + expect(contextOf(rollbar)).toBe('inner3'); + + unmount(); + expect(contextOf(rollbar)).toBe('root'); }); }); From 7eced8f96f400aa90e3e6b5f2927964194d7f788 Mon Sep 17 00:00:00 2001 From: Brian Rue Date: Fri, 25 Sep 2026 14:03:47 -0700 Subject: [PATCH 04/13] Key the context stack by order instead of mutating entries CI's examples lint (eslint-plugin-react-hooks 7) rejects the hook mutating the entry object it kept in useState ("Cannot modify local variables after render completes"). The stack now maps each order number to its context, and callers pass the context in, so nothing held by React is mutated. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/context-stack.js | 25 ++++++++++--------------- src/hooks/use-rollbar-context.js | 7 +++---- src/rollbar-context.js | 11 +++++++---- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/context-stack.js b/src/context-stack.js index 0d701a1..d29d27c 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -16,35 +16,30 @@ export function nextContextOrder() { } function applyInnermost(rollbar, stack) { - let innermost = null; - for (const entry of stack.entries) { - if (!innermost || entry.order > innermost.order) { - innermost = entry; - } - } - rollbar.configure({ payload: { context: innermost.context } }); + const innermost = Math.max(...stack.contexts.keys()); + rollbar.configure({ payload: { context: stack.contexts.get(innermost) } }); } -// `entry` is `{ order, context }`, owned by the caller. Adds it, or applies -// its new `context` if it's already there. -export function setContext(rollbar, entry) { +// Adds the context for `order`, or applies its new value if it's already +// there. +export function setContext(rollbar, order, context) { let stack = stacks.get(rollbar); if (!stack) { // rollbar.js has no default payload, so options.payload is undefined // unless the config sets it. - stack = { base: rollbar.options.payload?.context, entries: new Set() }; + stack = { base: rollbar.options.payload?.context, contexts: new Map() }; stacks.set(rollbar, stack); } - stack.entries.add(entry); + stack.contexts.set(order, context); applyInnermost(rollbar, stack); } -export function removeContext(rollbar, entry) { +export function removeContext(rollbar, order) { const stack = stacks.get(rollbar); - if (!stack || !stack.entries.delete(entry)) { + if (!stack || !stack.contexts.delete(order)) { return; } - if (stack.entries.size) { + if (stack.contexts.size) { applyInnermost(rollbar, stack); return; } diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index e5c6f8a..7ff680b 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -15,11 +15,10 @@ export function useRollbarContext(ctx = '', isLayout = false) { invariant(typeof ctx === 'string', '`ctx` must be a string'); const rollbar = useRollbar(); // Where this component sits among nested contexts; see context-stack.js. - const [entry] = useState(() => ({ order: nextContextOrder(), context: ctx })); + const [order] = useState(nextContextOrder); const useEffectOfType = isLayout ? useLayoutEffect : useEffect; useEffectOfType(() => { - entry.context = ctx; - setContext(rollbar, entry); + setContext(rollbar, order, ctx); }, [ctx]); - useEffectOfType(() => () => removeContext(rollbar, entry), []); + useEffectOfType(() => () => removeContext(rollbar, order), []); } diff --git a/src/rollbar-context.js b/src/rollbar-context.js index a4c679c..dd7a30c 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -21,13 +21,16 @@ export class RollbarContext extends Component { // Where this component sits among nested contexts; see context-stack.js. // Kept on the instance rather than in state because with onRender it is // added during render, where setState isn't allowed. - entry = { order: nextContextOrder(), context: undefined }; + order = nextContextOrder(); active = false; changeContext = () => { - this.entry.context = this.props.context; this.active = true; - setContext(getRollbarFromContext(this.context), this.entry); + setContext( + getRollbarFromContext(this.context), + this.order, + this.props.context, + ); }; componentDidMount() { @@ -45,7 +48,7 @@ export class RollbarContext extends Component { } componentWillUnmount() { - removeContext(getRollbarFromContext(this.context), this.entry); + removeContext(getRollbarFromContext(this.context), this.order); this.active = false; } From 751938b753661e095b47a4e67bd98d19e182997a Mon Sep 17 00:00:00 2001 From: Brian Rue Date: Fri, 25 Sep 2026 15:27:49 -0700 Subject: [PATCH 05/13] Don't leave an onRender context behind when React discards the render Review feedback: with onRender, the stack entry was added in render() and only removed in componentWillUnmount. When an ErrorBoundary around the RollbarContext catches an error from a child's first render, React never mounts the RollbarContext, so the entry stayed in the stack for good and outranked every other context. The same happened on the server, where nothing mounts. onRender now sets the context directly in render(), and the component joins the stack on mount. A microtask queued from render() applies the context of whatever is mounted again. React reports the error during the commit, before the microtask runs, and rollbar.js captures its options when rollbar.error() is called, so the report keeps this context. This also fixes the leak on main when no other context is mounted. The README now recommends putting RollbarContext outside the ErrorBoundary: React removes everything inside the boundary before the error is reported, so a RollbarContext inside it can't apply to errors after the first render. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 41 ++++--- src/context-stack.js | 66 ++++++---- src/rollbar-context.js | 20 ++-- src/tests/components/rollbar-context.test.tsx | 113 ++++++++++++++++-- 4 files changed, 184 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 82326bd..31decc4 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ next month. - [Pass a Fallback UI](#pass-a-fallback-ui) - [`RollbarContext` Component](#rollbarcontext-component) - [Basic Usage](#basic-usage) - - [Setting the context before children render](#setting-the-context-before-children-render) + - [Using with `ErrorBoundary`](#using-with-errorboundary) - [Using with React Router](#using-with-react-router) - [Functions](#functions) - [`historyContext` to create `history.listener`](#historycontext-to-create-historylistener) @@ -359,8 +359,7 @@ Use the `RollbarContext` component to declaratively set the `context` value used messages to [Rollbar]. The context applies to any log or message sent to [Rollbar] while the `RollbarContext` is mounted on the tree. For -errors that your `ErrorBoundary` from above catches while the children are first rendering, see -[Setting the context before children render](#setting-the-context-before-children-render). +errors that your `ErrorBoundary` from above catches, see [Using with `ErrorBoundary`](#using-with-errorboundary). Like `ErrorBoundary` above, `RollbarContext` relies on a [`Provider`] for an instance of a [Rollbar.js] client. @@ -378,30 +377,36 @@ function HomePage() { } ``` -#### Setting the context before children render +A change to the `context` prop is applied, and the previous context is restored on unmount. -By default `RollbarContext` sets the context when it mounts, which React does after the children have rendered and -mounted. An error thrown while they're first rendering, which is what an `ErrorBoundary` usually catches, is -therefore reported with the previous context. +`RollbarContext` components can be nested, including with the `useRollbarContext` hook. The innermost one that's +mounted sets the context. When it unmounts, the next one out applies again, with its current `context`. + +#### Using with `ErrorBoundary` -Add the `onRender` prop to set the context during the first render instead, before the children render: +Put the `RollbarContext` outside the `ErrorBoundary`, and add the `onRender` prop: ```javascript - - + + - - + + ``` -Either way, a change to the `context` prop is applied, and the previous context is restored on unmount. +Outside, because when the `ErrorBoundary` catches an error, React removes everything inside it before the error is +reported. A `RollbarContext` inside it has already been removed by then, so an error thrown after the first render +is reported with the previous context. -`RollbarContext` components can be nested, including with the `useRollbarContext` hook. The innermost one that's -mounted sets the context. When it unmounts, the next one out applies again, with its current `context`. +`onRender`, because by default `RollbarContext` sets the context when it mounts, which React does after the children +have rendered and mounted. An error thrown while they're first rendering would be reported with the previous +context. With `onRender`, `RollbarContext` sets the context during its first render, before the children render. -`onRender` sets the context from inside `render`. If React throws that render away without committing it, for -example during a transition that gets interrupted, nothing restores the previous context, so the context can stay -set to a page that never showed. That's why it isn't the default. +That means the context is set before React has committed anything. React can throw the render away, for example +when an `ErrorBoundary` around the `RollbarContext` catches an error, and nothing mounts when rendering on the +server. So once React has finished, and an `ErrorBoundary` has reported the error, `RollbarContext` puts back the +context of whatever is mounted. Until then, anything else sent to [Rollbar] also gets this context. That's why +`onRender` isn't the default. #### Using with React Router diff --git a/src/context-stack.js b/src/context-stack.js index d29d27c..d8d505f 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -1,7 +1,7 @@ -// The RollbarContext components and useRollbarContext hooks that are -// currently setting a context, per Rollbar client. The innermost one decides -// the client's context. When the last one goes away, the context from before -// the first one is restored. +// The RollbarContext components and useRollbarContext hooks that are mounted +// and setting a context, per Rollbar client. The innermost one decides the +// client's context. When the last one goes away, the context from before the +// first one is restored. const stacks = new WeakMap(); let lastOrder = 0; @@ -15,14 +15,7 @@ export function nextContextOrder() { return lastOrder; } -function applyInnermost(rollbar, stack) { - const innermost = Math.max(...stack.contexts.keys()); - rollbar.configure({ payload: { context: stack.contexts.get(innermost) } }); -} - -// Adds the context for `order`, or applies its new value if it's already -// there. -export function setContext(rollbar, order, context) { +function getStack(rollbar) { let stack = stacks.get(rollbar); if (!stack) { // rollbar.js has no default payload, so options.payload is undefined @@ -30,17 +23,13 @@ export function setContext(rollbar, order, context) { stack = { base: rollbar.options.payload?.context, contexts: new Map() }; stacks.set(rollbar, stack); } - stack.contexts.set(order, context); - applyInnermost(rollbar, stack); + return stack; } -export function removeContext(rollbar, order) { - const stack = stacks.get(rollbar); - if (!stack || !stack.contexts.delete(order)) { - return; - } +function applyStack(rollbar, stack) { if (stack.contexts.size) { - applyInnermost(rollbar, stack); + const innermost = Math.max(...stack.contexts.keys()); + rollbar.configure({ payload: { context: stack.contexts.get(innermost) } }); return; } stacks.delete(rollbar); @@ -48,3 +37,40 @@ export function removeContext(rollbar, order) { // needs ''. rollbar.js sends '' for an unset context anyway. rollbar.configure({ payload: { context: stack.base ?? '' } }); } + +// Adds the context for `order`, or applies its new value if it's already +// there. +export function setContext(rollbar, order, context) { + const stack = getStack(rollbar); + stack.contexts.set(order, context); + applyStack(rollbar, stack); +} + +export function removeContext(rollbar, order) { + const stack = stacks.get(rollbar); + if (stack?.contexts.delete(order)) { + applyStack(rollbar, stack); + } +} + +// For RollbarContext's onRender: sets `context` while the component renders, +// before it has mounted and been added with setContext. React can throw that +// render away without mounting anything, for example when an ErrorBoundary +// around the component catches an error from its children, and nothing mounts +// on the server. So a microtask applies the context of whatever is mounted +// again. React commits in the same task that it finishes rendering in, and an +// ErrorBoundary reports during the commit, so the microtask runs after both. +// If a transition yields partway through rendering, the microtask runs then, +// but when a child throws, React renders again from the start, synchronously, +// before it commits. +export function setRenderContext(rollbar, context) { + // Taken before the context changes, so that it's the one restored if + // nothing is mounted. + const stack = getStack(rollbar); + rollbar.configure({ payload: { context } }); + Promise.resolve().then(() => { + if (stacks.get(rollbar) === stack) { + applyStack(rollbar, stack); + } + }); +} diff --git a/src/rollbar-context.js b/src/rollbar-context.js index dd7a30c..a4ff7cb 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -3,7 +3,12 @@ import { Component } from 'react'; import PropTypes from 'prop-types'; import { Context, getRollbarFromContext } from './provider'; -import { nextContextOrder, removeContext, setContext } from './context-stack'; +import { + nextContextOrder, + removeContext, + setContext, + setRenderContext, +} from './context-stack'; export class RollbarContext extends Component { static propTypes = { @@ -19,8 +24,6 @@ export class RollbarContext extends Component { static contextType = Context; // Where this component sits among nested contexts; see context-stack.js. - // Kept on the instance rather than in state because with onRender it is - // added during render, where setState isn't allowed. order = nextContextOrder(); active = false; @@ -34,10 +37,7 @@ export class RollbarContext extends Component { }; componentDidMount() { - // With onRender the context was already set during the first render. - if (!this.active) { - this.changeContext(); - } + this.changeContext(); } componentDidUpdate(prevProps) { @@ -53,9 +53,11 @@ export class RollbarContext extends Component { } render() { - const { onRender } = this.props; + const { onRender, context } = this.props; if (onRender && !this.active) { - this.changeContext(); + // Before the children render, so that errors they throw are reported + // with this context. The component is added to the stack on mount. + setRenderContext(getRollbarFromContext(this.context), context); } return this.props.children; } diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index d58dd36..75eb04f 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { renderToString } from 'react-dom/server'; import { render } from '@testing-library/react'; import Rollbar from 'rollbar'; import { @@ -18,6 +19,13 @@ const makeRollbar = (config: Rollbar.Configuration = {}) => const contextOf = (rollbar: Rollbar): unknown => rollbar.options.payload?.context; +// With onRender, RollbarContext puts back the context of whatever is mounted +// in a microtask after rendering. +const afterMicrotasks = () => + new Promise((resolve) => { + setTimeout(resolve, 0); + }); + describe('RollbarContext', () => { let consoleError: jest.SpyInstance; @@ -76,18 +84,25 @@ describe('RollbarContext', () => { // #88 describe('when a child throws while rendering', () => { - const renderThrowing = (onRender: boolean) => { + const makeReporting = () => { const rollbar = makeRollbar({ payload: { context: 'root' } }); const reported: unknown[] = []; rollbar.error = jest.fn(() => { reported.push(contextOf(rollbar)); return { uuid: '' }; }); - const Throw = () => { + return { rollbar, reported }; + }; + const Throw = ({ when = true }: { when?: boolean }) => { + if (when) { throw new Error('render error'); - }; + } + return null; + }; + const renderThrowing = (onRender: boolean) => { + const reporting = makeReporting(); render( - + @@ -95,16 +110,74 @@ describe('RollbarContext', () => { , ); - expect(rollbar.error).toHaveBeenCalledTimes(1); - return reported[0]; + return reporting; }; it('reports with the previous context by default', () => { - expect(renderThrowing(false)).toBe('root'); + expect(renderThrowing(false).reported).toEqual(['root']); + }); + + it('reports with this context when onRender is set', async () => { + const { rollbar, reported } = renderThrowing(true); + expect(reported).toEqual(['home']); + + // The ErrorBoundary replaced the RollbarContext before it mounted. + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('root'); }); - it('reports with this context when onRender is set', () => { - expect(renderThrowing(true)).toBe('home'); + it('leaves nothing behind when the ErrorBoundary replaces an onRender context', async () => { + const { rollbar, reported } = makeReporting(); + const ui = ( + outer: string, + page: 'throws' | 'renders' | 'none', + boundaryKey: number, + ) => ( + + + + {page !== 'none' && ( + + + + )} + + + + ); + + const { rerender } = render(ui('app', 'throws', 1)); + expect(reported).toEqual(['home']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('app'); + + rerender(ui('app2', 'throws', 1)); + expect(contextOf(rollbar)).toBe('app2'); + + // A new ErrorBoundary, and this time the page renders. + rerender(ui('app2', 'renders', 2)); + expect(contextOf(rollbar)).toBe('home'); + + rerender(ui('app2', 'none', 2)); + expect(contextOf(rollbar)).toBe('app2'); + }); + + it('reports with this context from outside the ErrorBoundary, on first render and after', () => { + const { rollbar, reported } = makeReporting(); + const ui = (throws: boolean, boundaryKey: number) => ( + + + + + + + + ); + + const { rerender } = render(ui(true, 1)); + rerender(ui(false, 2)); + rerender(ui(true, 2)); + expect(reported).toEqual(['home', 'home']); }); }); @@ -130,6 +203,28 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('root'); }); + it('puts the context back after rendering on the server', async () => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + let seen: unknown; + const Child = () => { + seen = contextOf(rollbar); + return null; + }; + + renderToString( + + + + + , + ); + expect(seen).toBe('home'); + + // Nothing mounts on the server. + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('root'); + }); + it('does not call setState during render', () => { const rollbar = makeRollbar(); render( From abca1d9666a2b4259b0e97abfcfe482645076f3f Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 25 Sep 2026 23:02:48 +0000 Subject: [PATCH 06/13] Correct the note on restoring '' for an unset context on the server Restoring an unset context as '' is only equivalent to leaving it unset in the browser. On the server, rollbar.js derives the context from the Express route in addRequestData, and addPayloadOptions then merges payload.context, including '', over it. So after an onRender server render with a shared instance whose config has no payload.context, later request errors lose their route context. rollbar.js has no way to remove the key (configure() skips undefined and the notifier keeps its own merged copy), so correct the comment and document it in the README instead. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++++ src/context-stack.js | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 31decc4..ad451d4 100644 --- a/README.md +++ b/README.md @@ -408,6 +408,10 @@ server. So once React has finished, and an `ErrorBoundary` has reported the erro context of whatever is mounted. Until then, anything else sent to [Rollbar] also gets this context. That's why `onRender` isn't the default. +On the server, give the `Provider` a `config` rather than a shared `instance`. If the instance's config doesn't +set `payload.context`, `RollbarContext` can only put back an empty context, and on the server that replaces the +context rollbar.js takes from the request's route for later errors. + #### Using with React Router It's useful to set the `context` in [Rollbar] associated with areas of your application. On the server it's usually diff --git a/src/context-stack.js b/src/context-stack.js index d8d505f..de5083d 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -33,8 +33,12 @@ function applyStack(rollbar, stack) { return; } stacks.delete(rollbar); - // configure() ignores undefined values, so restoring an unset context - // needs ''. rollbar.js sends '' for an unset context anyway. + // configure() ignores undefined values and there's no way to remove the key, + // so restoring an unset context needs ''. In the browser that's sent the same + // as an unset context. On the server it isn't: rollbar.js takes the context + // from the request's route, and payload.context, even '', replaces it. Only + // onRender restores on the server, since nothing mounts there; the README + // covers this. rollbar.configure({ payload: { context: stack.base ?? '' } }); } From ec425b08e2f9b6fd16ae3054e8134c41b4e02747 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Sun, 27 Sep 2026 04:13:28 +0000 Subject: [PATCH 07/13] Keep an onRender context in the stack until React commits it The context onRender set during render wasn't recorded in the context stack, so any other stack change earlier in the same commit replaced it before an ErrorBoundary inside reported. That happens on a page change, when the old page's RollbarContext unmounts in the mutation phase, and when an earlier sibling RollbarContext re-applies in componentDidUpdate. A changed context prop on a mounted onRender instance also wasn't set until componentDidUpdate, after the boundary had reported. setRenderContext now adds a rendering entry for the component's order, which ranks with the mounted ones. The component's mount or update replaces it, and a microtask removes it if the render was thrown away. render() sets it on the first render and whenever the context prop differs from the one applied on mount or update. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- src/context-stack.js | 58 ++++++++---- src/rollbar-context.js | 20 ++-- src/tests/components/rollbar-context.test.tsx | 92 +++++++++++++++++++ 4 files changed, 147 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index ad451d4..6bc8d7b 100644 --- a/README.md +++ b/README.md @@ -400,7 +400,9 @@ is reported with the previous context. `onRender`, because by default `RollbarContext` sets the context when it mounts, which React does after the children have rendered and mounted. An error thrown while they're first rendering would be reported with the previous -context. With `onRender`, `RollbarContext` sets the context during its first render, before the children render. +context. With `onRender`, `RollbarContext` sets the context during its first render, before the children render, +and again whenever the `context` prop changes, so a page that React mounts inside an existing `RollbarContext` is +covered too. That means the context is set before React has committed anything. React can throw the render away, for example when an `ErrorBoundary` around the `RollbarContext` catches an error, and nothing mounts when rendering on the diff --git a/src/context-stack.js b/src/context-stack.js index de5083d..c2cbb82 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -20,16 +20,27 @@ function getStack(rollbar) { if (!stack) { // rollbar.js has no default payload, so options.payload is undefined // unless the config sets it. - stack = { base: rollbar.options.payload?.context, contexts: new Map() }; + stack = { + base: rollbar.options.payload?.context, + contexts: new Map(), + // From setRenderContext: set during render, until React has committed. + rendering: new Map(), + }; stacks.set(rollbar, stack); } return stack; } function applyStack(rollbar, stack) { - if (stack.contexts.size) { - const innermost = Math.max(...stack.contexts.keys()); - rollbar.configure({ payload: { context: stack.contexts.get(innermost) } }); + const { contexts, rendering } = stack; + if (contexts.size || rendering.size) { + const innermost = Math.max(...contexts.keys(), ...rendering.keys()); + // A component that's rendering with a new context has the old one in + // `contexts` until it updates. + const context = rendering.has(innermost) + ? rendering.get(innermost).context + : contexts.get(innermost); + rollbar.configure({ payload: { context } }); return; } stacks.delete(rollbar); @@ -43,9 +54,11 @@ function applyStack(rollbar, stack) { } // Adds the context for `order`, or applies its new value if it's already -// there. +// there. It replaces any context setRenderContext set for `order`: the +// component has mounted or updated, so that render was committed. export function setContext(rollbar, order, context) { const stack = getStack(rollbar); + stack.rendering.delete(order); stack.contexts.set(order, context); applyStack(rollbar, stack); } @@ -57,23 +70,30 @@ export function removeContext(rollbar, order) { } } -// For RollbarContext's onRender: sets `context` while the component renders, -// before it has mounted and been added with setContext. React can throw that -// render away without mounting anything, for example when an ErrorBoundary -// around the component catches an error from its children, and nothing mounts -// on the server. So a microtask applies the context of whatever is mounted -// again. React commits in the same task that it finishes rendering in, and an -// ErrorBoundary reports during the commit, so the microtask runs after both. -// If a transition yields partway through rendering, the microtask runs then, -// but when a child throws, React renders again from the start, synchronously, +// For RollbarContext's onRender: sets `context` for `order` while the +// component renders, before it has mounted or updated and called setContext. +// It's kept in the stack so that anything else applied before React finishes, +// like another context unmounting or updating in the same commit, doesn't +// replace it before an ErrorBoundary inside reports. +// +// React can throw the render away without committing it, for example when an +// ErrorBoundary around the component catches an error from its children, and +// nothing mounts on the server. So if the component hasn't mounted or updated +// by then, a microtask removes it again, leaving whatever did. React commits +// in the same task that it finishes rendering in, and an ErrorBoundary +// reports during the commit, so the microtask runs after both. If a +// transition yields partway through rendering, the microtask runs then, but +// when a child throws, React renders again from the start, synchronously, // before it commits. -export function setRenderContext(rollbar, context) { - // Taken before the context changes, so that it's the one restored if - // nothing is mounted. +export function setRenderContext(rollbar, order, context) { const stack = getStack(rollbar); - rollbar.configure({ payload: { context } }); + const entry = { context }; + stack.rendering.set(order, entry); + applyStack(rollbar, stack); Promise.resolve().then(() => { - if (stacks.get(rollbar) === stack) { + // Unless a later render replaced it; its own microtask removes that. + if (stack.rendering.get(order) === entry) { + stack.rendering.delete(order); applyStack(rollbar, stack); } }); diff --git a/src/rollbar-context.js b/src/rollbar-context.js index a4ff7cb..b190649 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -25,10 +25,11 @@ export class RollbarContext extends Component { // Where this component sits among nested contexts; see context-stack.js. order = nextContextOrder(); - active = false; + // The context this component has set since mounting, if it's mounted. + mountedContext = undefined; changeContext = () => { - this.active = true; + this.mountedContext = this.props.context; setContext( getRollbarFromContext(this.context), this.order, @@ -49,15 +50,20 @@ export class RollbarContext extends Component { componentWillUnmount() { removeContext(getRollbarFromContext(this.context), this.order); - this.active = false; + this.mountedContext = undefined; } render() { const { onRender, context } = this.props; - if (onRender && !this.active) { - // Before the children render, so that errors they throw are reported - // with this context. The component is added to the stack on mount. - setRenderContext(getRollbarFromContext(this.context), context); + if (onRender && context !== this.mountedContext) { + // Before the children render, on the first render and when the context + // prop changes, so that errors they throw are reported with this + // context. It's applied for good on mount or update. + setRenderContext( + getRollbarFromContext(this.context), + this.order, + context, + ); } return this.props.children; } diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index 75eb04f..9e87c53 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -179,6 +179,98 @@ describe('RollbarContext', () => { rerender(ui(true, 2)); expect(reported).toEqual(['home', 'home']); }); + + // React unmounts the old page before the new page's ErrorBoundary reports, + // in the same commit. + it('reports with the new page context when the old page unmounts in the same commit', async () => { + const { rollbar, reported } = makeReporting(); + const Page = ({ name, throws }: { name: string; throws: boolean }) => ( + + + + + + ); + const ui = (page: string) => ( + + + + ); + + const { rerender } = render(ui('home')); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home'); + + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('about'); + }); + + // An earlier sibling's componentDidUpdate runs before the ErrorBoundary + // reports, in the same commit. + it('reports with this context when a sibling context updates in the same commit', async () => { + const { rollbar, reported } = makeReporting(); + const ui = (showPage: boolean) => ( + + +
+ + {showPage && ( + + + + + + )} + + ); + + const { rerender } = render(ui(false)); + rerender(ui(true)); + expect(reported).toEqual(['home']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home'); + }); + + it('reports with the new context when the context prop changes', async () => { + const { rollbar, reported } = makeReporting(); + // React reuses the RollbarContext and mounts a new page inside it. + const ui = (page: string) => ( + + + + + + + + ); + + const { rerender } = render(ui('home')); + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('about'); + }); + + it('leaves nothing behind when the ErrorBoundary replaces an updated onRender context', async () => { + const { rollbar, reported } = makeReporting(); + const ui = (page: string) => ( + + + + + + + + ); + + const { rerender } = render(ui('home')); + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('root'); + }); }); describe('with onRender', () => { From 8bfdfc20e9e5f05efe4bb24e834aa770407506cf Mon Sep 17 00:00:00 2001 From: AI Agent Date: Sun, 27 Sep 2026 05:27:18 +0000 Subject: [PATCH 08/13] Remove a useRollbarContext context before an ErrorBoundary reports By default the hook removed its context in a passive effect cleanup. When React removes a component, it runs passive cleanups only after the commit, while an ErrorBoundary reports during it. So with the README's pattern, a page that used the hook still set the context of an error the next page threw on its first render, since the hook ranks inside the RollbarContext. The hook now always removes its context in a layout effect cleanup, which React runs during the commit, whatever isLayout is. React 18 and later also run layout cleanups when Suspense hides a tree that was showing, and only rerun layout effects when it shows it again, so that effect puts back the context the hook last set. On the server it uses useEffect, since useLayoutEffect warns there before React 19. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- src/hooks/use-rollbar-context.js | 38 ++++++++++++++++--- .../rollbar-context.server.test.tsx | 31 +++++++++++++++ src/tests/components/rollbar-context.test.tsx | 28 ++++++++++++++ 4 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 src/tests/components/rollbar-context.server.test.tsx diff --git a/README.md b/README.md index 6bc8d7b..49c23fb 100644 --- a/README.md +++ b/README.md @@ -593,7 +593,9 @@ As an alternative to the [`RollbarContext`] component, you can use the `useRollb to set the `context` in the [Rollbar.js] client provided by the [`Provider`] above in the React Tree. The hook sets the context in an effect, so like `RollbarContext` without `onRender`, it doesn't apply to errors -thrown while the component and its children are first rendering. +thrown while the component and its children are first rendering. It's removed as soon as React removes the component, +though, so in the [`ErrorBoundary` pattern](#using-with-errorboundary) a page that used it doesn't set the context of +an error the next page throws. Here's an example of using it in several components: diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index 7ff680b..ef8a2f0 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -1,10 +1,19 @@ 'use client'; import invariant from 'tiny-invariant'; -import { useEffect, useLayoutEffect, useState } from 'react'; +import { useEffect, useLayoutEffect, useRef, useState } from 'react'; import { useRollbar } from './use-rollbar'; import { nextContextOrder, removeContext, setContext } from '../context-stack'; +// Before React 19, useLayoutEffect warns when rendering on the server, where +// no effects run anyway. +const useClientLayoutEffect = + typeof window === 'undefined' ? useEffect : useLayoutEffect; + +function useEffectOfType(isLayout, effect, deps) { + (isLayout ? useLayoutEffect : useEffect)(effect, deps); +} + // Simple version does its job // export function useRollbarContext(context) { // useRollbarConfiguration({ payload: { context }}); @@ -16,9 +25,26 @@ export function useRollbarContext(ctx = '', isLayout = false) { const rollbar = useRollbar(); // Where this component sits among nested contexts; see context-stack.js. const [order] = useState(nextContextOrder); - const useEffectOfType = isLayout ? useLayoutEffect : useEffect; - useEffectOfType(() => { - setContext(rollbar, order, ctx); - }, [ctx]); - useEffectOfType(() => () => removeContext(rollbar, order), []); + // The context this hook has set, once it has. + const appliedRef = useRef(); + // Removed in a layout effect cleanup, whatever `isLayout` is. When React + // removes a component, it runs its layout cleanups during the commit, but + // its passive ones only after it, so after an ErrorBoundary elsewhere in the + // tree has reported an error thrown in that commit, like the next page's. + useClientLayoutEffect(() => { + // React 18 and later also run layout cleanups when Suspense hides a tree + // that was showing, and run only layout effects again when it shows it. + if (appliedRef.current !== undefined) { + setContext(rollbar, order, appliedRef.current); + } + return () => removeContext(rollbar, order); + }, []); + useEffectOfType( + isLayout, + () => { + appliedRef.current = ctx; + setContext(rollbar, order, ctx); + }, + [ctx], + ); } diff --git a/src/tests/components/rollbar-context.server.test.tsx b/src/tests/components/rollbar-context.server.test.tsx new file mode 100644 index 0000000..90edb41 --- /dev/null +++ b/src/tests/components/rollbar-context.server.test.tsx @@ -0,0 +1,31 @@ +/** + * @jest-environment node + */ +import React from 'react'; +import { renderToString } from 'react-dom/server'; +import Rollbar from 'rollbar'; +import { Provider, useRollbarContext } from '../rollbar-react'; + +describe('useRollbarContext on the server', () => { + it('renders without the useLayoutEffect warning', () => { + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + const rollbar = new Rollbar({ + accessToken: 'POST_SERVER_ITEM_TOKEN', + enabled: false, + }); + const Page = () => { + useRollbarContext('home'); + return null; + }; + + renderToString( + + + , + ); + expect(consoleError).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }); +}); diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index 9e87c53..3116852 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -207,6 +207,34 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('about'); }); + // React runs passive effect cleanups after the commit in which the + // ErrorBoundary reports, so the old page's hook has to leave before then. + it('reports with the new page context when the old page used useRollbarContext', async () => { + const { rollbar, reported } = makeReporting(); + const HomePage = () => { + useRollbarContext('home#index'); + return null; + }; + const ui = (page: string) => ( + + + + {page === 'home' ? : } + + + + ); + + const { rerender } = render(ui('home')); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home#index'); + + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('about'); + }); + // An earlier sibling's componentDidUpdate runs before the ErrorBoundary // reports, in the same commit. it('reports with this context when a sibling context updates in the same commit', async () => { From 9677a34efcd2abd523e70f97562d3f1426b436da Mon Sep 17 00:00:00 2001 From: AI Agent Date: Sun, 27 Sep 2026 06:11:52 +0000 Subject: [PATCH 09/13] Avoid the server useLayoutEffect warning with useRollbarContext(ctx, true) useEffectOfType still passed the raw useLayoutEffect when isLayout was true, so the hook logged "useLayoutEffect does nothing on the server" under react-dom 17's server renderer. Use useClientLayoutEffect there too, and run the server test with both isLayout values. Co-Authored-By: Claude Opus 5.5 --- src/hooks/use-rollbar-context.js | 2 +- .../rollbar-context.server.test.tsx | 43 ++++++++++--------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index ef8a2f0..b6c94e1 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -11,7 +11,7 @@ const useClientLayoutEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect; function useEffectOfType(isLayout, effect, deps) { - (isLayout ? useLayoutEffect : useEffect)(effect, deps); + (isLayout ? useClientLayoutEffect : useEffect)(effect, deps); } // Simple version does its job diff --git a/src/tests/components/rollbar-context.server.test.tsx b/src/tests/components/rollbar-context.server.test.tsx index 90edb41..2d62542 100644 --- a/src/tests/components/rollbar-context.server.test.tsx +++ b/src/tests/components/rollbar-context.server.test.tsx @@ -7,25 +7,28 @@ import Rollbar from 'rollbar'; import { Provider, useRollbarContext } from '../rollbar-react'; describe('useRollbarContext on the server', () => { - it('renders without the useLayoutEffect warning', () => { - const consoleError = jest - .spyOn(console, 'error') - .mockImplementation(() => {}); - const rollbar = new Rollbar({ - accessToken: 'POST_SERVER_ITEM_TOKEN', - enabled: false, - }); - const Page = () => { - useRollbarContext('home'); - return null; - }; + it.each([false, true])( + 'renders without the useLayoutEffect warning (isLayout: %s)', + (isLayout) => { + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + const rollbar = new Rollbar({ + accessToken: 'POST_SERVER_ITEM_TOKEN', + enabled: false, + }); + const Page = () => { + useRollbarContext('home', isLayout); + return null; + }; - renderToString( - - - , - ); - expect(consoleError).not.toHaveBeenCalled(); - consoleError.mockRestore(); - }); + renderToString( + + + , + ); + expect(consoleError).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }, + ); }); From dcac60112a267b685a398cc3819755323620700d Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 9 Oct 2026 21:47:38 +0000 Subject: [PATCH 10/13] Report ErrorBoundary errors with the nearest RollbarContext's context Addresses waltjones's review on #166. RollbarContext now also passes its context down through a React context, and ErrorBoundary applies it around the report in componentDidCatch, then puts the previous context back. React resolves the context during render, so this no longer depends on when React commits. It works without onRender, on the first render and after a change to the `context` prop. It also fixes React 19 holding a commit back for a precedence stylesheet, where the onRender microtask removed the context before the report. A useRollbarContext between the RollbarContext and the ErrorBoundary still wins, as the innermost context. Without a RollbarContext, ErrorBoundary doesn't touch the client, as before. - onRender stays, for anything else the children send while mounting. The README documents it with that use, and its caveats now include the React 19 delayed commit. The context-stack.js comment no longer claims React always commits in the same task. - componentDidUpdate only re-applies when the `context` prop changes, with or without onRender. - RollbarContext without children no longer throws on React 17. - jest-setup.ts formats console.error's arguments, so a prop-type failure fails the test again. React passes the message as a format string. The RollbarContext tests' console.error mock passes those failures on, and the server test restores its spy in afterEach. Co-Authored-By: Claude Opus 5.5 --- README.md | 62 +-- src/context-stack.js | 58 ++- src/error-boundary.js | 37 +- src/rollbar-context.js | 21 +- .../rollbar-context.server.test.tsx | 14 +- src/tests/components/rollbar-context.test.tsx | 352 ++++++++++++------ src/tests/jest-setup.ts | 9 +- 7 files changed, 382 insertions(+), 171 deletions(-) diff --git a/README.md b/README.md index 49c23fb..bd854ba 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,7 @@ next month. - [`RollbarContext` Component](#rollbarcontext-component) - [Basic Usage](#basic-usage) - [Using with `ErrorBoundary`](#using-with-errorboundary) + - [Setting the context before children render](#setting-the-context-before-children-render) - [Using with React Router](#using-with-react-router) - [Functions](#functions) - [`historyContext` to create `history.listener`](#historycontext-to-create-historylistener) @@ -358,8 +359,8 @@ export function App(props) { Use the `RollbarContext` component to declaratively set the `context` value used by [Rollbar.js] when it's sending any messages to [Rollbar]. -The context applies to any log or message sent to [Rollbar] while the `RollbarContext` is mounted on the tree. For -errors that your `ErrorBoundary` from above catches, see [Using with `ErrorBoundary`](#using-with-errorboundary). +The context applies to any log or message sent to [Rollbar] while the `RollbarContext` is mounted on the tree, and to +errors that your `ErrorBoundary` from above catches inside it; see [Using with `ErrorBoundary`](#using-with-errorboundary). Like `ErrorBoundary` above, `RollbarContext` relies on a [`Provider`] for an instance of a [Rollbar.js] client. @@ -384,35 +385,49 @@ mounted sets the context. When it unmounts, the next one out applies again, with #### Using with `ErrorBoundary` -Put the `RollbarContext` outside the `ErrorBoundary`, and add the `onRender` prop: +Put the `RollbarContext` outside the `ErrorBoundary`: ```javascript - + ``` +The `ErrorBoundary` reports an error with the context of the nearest `RollbarContext` around it. That includes an +error thrown while they're first rendering, before the `RollbarContext` has mounted, and an error thrown after a +change to the `context` prop, before it has been applied. A `useRollbarContext` hook between the two that has set its +context still takes precedence, as the innermost one. + Outside, because when the `ErrorBoundary` catches an error, React removes everything inside it before the error is -reported. A `RollbarContext` inside it has already been removed by then, so an error thrown after the first render -is reported with the previous context. +reported. A `RollbarContext` inside it has already been removed by then. + +#### Setting the context before children render + +By default, `RollbarContext` sets the context when it mounts, which React does after the children have rendered and +mounted. Apart from errors that an `ErrorBoundary` inside it catches, anything sent to [Rollbar] before then, for +example from the children's own effects when they mount, gets the previous context. The `onRender` prop sets the +context during render instead, before the children render, on the first render and whenever the `context` prop +changes: -`onRender`, because by default `RollbarContext` sets the context when it mounts, which React does after the children -have rendered and mounted. An error thrown while they're first rendering would be reported with the previous -context. With `onRender`, `RollbarContext` sets the context during its first render, before the children render, -and again whenever the `context` prop changes, so a page that React mounts inside an existing `RollbarContext` is -covered too. +```javascript + + + +``` -That means the context is set before React has committed anything. React can throw the render away, for example -when an `ErrorBoundary` around the `RollbarContext` catches an error, and nothing mounts when rendering on the -server. So once React has finished, and an `ErrorBoundary` has reported the error, `RollbarContext` puts back the -context of whatever is mounted. Until then, anything else sent to [Rollbar] also gets this context. That's why -`onRender` isn't the default. +That means the context is set before React has committed anything, which is why `onRender` isn't the default: -On the server, give the `Provider` a `config` rather than a shared `instance`. If the instance's config doesn't -set `payload.context`, `RollbarContext` can only put back an empty context, and on the server that replaces the -context rollbar.js takes from the request's route for later errors. +- Until React commits, anything else sent to [Rollbar] also gets this context. +- React can throw the render away, for example when an `ErrorBoundary` around the `RollbarContext` catches an error, + and nothing mounts when rendering on the server. So in a microtask after rendering, `RollbarContext` puts back the + context of whatever is mounted. React usually commits before then, but not always: a transition can yield partway + through rendering, and React 19 can hold a commit back until a `` in it has + loaded. The children then mount with the previous context, as without `onRender`. +- On the server, give the `Provider` a `config` rather than a shared `instance`. If the instance's config doesn't set + `payload.context`, `RollbarContext` can only put back an empty context, and on the server that replaces the context + rollbar.js takes from the request's route for later errors. #### Using with React Router @@ -592,10 +607,11 @@ function ContactDetails({ contactId }) { As an alternative to the [`RollbarContext`] component, you can use the `useRollbarContext` hook in your [Functional Component] to set the `context` in the [Rollbar.js] client provided by the [`Provider`] above in the React Tree. -The hook sets the context in an effect, so like `RollbarContext` without `onRender`, it doesn't apply to errors -thrown while the component and its children are first rendering. It's removed as soon as React removes the component, -though, so in the [`ErrorBoundary` pattern](#using-with-errorboundary) a page that used it doesn't set the context of -an error the next page throws. +The hook sets the context in an effect, so like `RollbarContext` without `onRender`, it doesn't apply to anything sent +while the component and its children are first rendering and mounting, and an `ErrorBoundary` inside the component +can't report with it then either. It's removed as soon as React removes the component, though, so in the +[`ErrorBoundary` pattern](#using-with-errorboundary) a page that used it doesn't set the context of an error the next +page throws. Here's an example of using it in several components: diff --git a/src/context-stack.js b/src/context-stack.js index c2cbb82..c4a2929 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -31,10 +31,14 @@ function getStack(rollbar) { return stack; } +function innermostOrder({ contexts, rendering }) { + return Math.max(...contexts.keys(), ...rendering.keys()); +} + function applyStack(rollbar, stack) { const { contexts, rendering } = stack; if (contexts.size || rendering.size) { - const innermost = Math.max(...contexts.keys(), ...rendering.keys()); + const innermost = innermostOrder(stack); // A component that's rendering with a new context has the old one in // `contexts` until it updates. const context = rendering.has(innermost) @@ -74,17 +78,18 @@ export function removeContext(rollbar, order) { // component renders, before it has mounted or updated and called setContext. // It's kept in the stack so that anything else applied before React finishes, // like another context unmounting or updating in the same commit, doesn't -// replace it before an ErrorBoundary inside reports. +// replace it before the children have mounted. // // React can throw the render away without committing it, for example when an // ErrorBoundary around the component catches an error from its children, and // nothing mounts on the server. So if the component hasn't mounted or updated -// by then, a microtask removes it again, leaving whatever did. React commits -// in the same task that it finishes rendering in, and an ErrorBoundary -// reports during the commit, so the microtask runs after both. If a -// transition yields partway through rendering, the microtask runs then, but -// when a child throws, React renders again from the start, synchronously, -// before it commits. +// by then, a microtask removes it again, leaving whatever did. When React +// commits in the same task that it finished rendering in, that's after the +// commit. React doesn't always: a transition can yield partway through +// rendering, and React 19 can hold a commit back until a stylesheet in it has +// loaded. Then the microtask runs first, and the children mount with the +// previous context, as without onRender. ErrorBoundary doesn't rely on this; +// see reportWithContext. export function setRenderContext(rollbar, order, context) { const stack = getStack(rollbar); const entry = { context }; @@ -98,3 +103,40 @@ export function setRenderContext(rollbar, order, context) { } }); } + +// For ErrorBoundary: calls `report` with the context of the nearest +// RollbarContext around the ErrorBoundary, `reportContext`, which React +// resolved while rendering it. That RollbarContext sets its context when it +// mounts or updates, which React does after the ErrorBoundary inside has +// reported, so on its first render or a change to its `context` prop the +// client still has the previous context. Under onRender, React may also have +// committed after the microtask in setRenderContext. +// +// rollbar.js has no per-item context, so the context is applied around the +// report: rollbar.js takes the options an item is sent with when it's logged, +// and configure() replaces them rather than changing them. An entry that ranks +// after the RollbarContext, like a useRollbarContext between it and the +// ErrorBoundary, keeps the context it set. +export function reportWithContext(rollbar, reportContext, report) { + // Without a RollbarContext, the client isn't touched, as before. + if (!reportContext) { + report(); + return; + } + const stack = stacks.get(rollbar); + const previous = rollbar.options.payload?.context; + if ( + (stack && innermostOrder(stack) > reportContext.order) || + reportContext.context === previous + ) { + report(); + return; + } + rollbar.configure({ payload: { context: reportContext.context } }); + try { + report(); + } finally { + // As in applyStack, configure() ignores undefined. + rollbar.configure({ payload: { context: previous ?? '' } }); + } +} diff --git a/src/error-boundary.js b/src/error-boundary.js index afbbc7a..2df31fc 100644 --- a/src/error-boundary.js +++ b/src/error-boundary.js @@ -5,6 +5,8 @@ import PropTypes from 'prop-types'; import invariant from 'tiny-invariant'; import { LEVEL_ERROR } from './constants'; import { Context, getRollbarFromContext } from './provider'; +import { ReportContext } from './rollbar-context'; +import { reportWithContext } from './context-stack'; import * as utils from './utils'; const INITIAL_ERROR_STATE = { hasError: false, error: null }; @@ -44,12 +46,14 @@ export class ErrorBoundary extends Component { const data = { ...info, ...custom }; const level = utils.value(targetLevel, LEVEL_ERROR, error, info); const rollbar = getRollbarFromContext(this.context); - if (!errorMessage) { - rollbar[level](error, data, callback); - } else { - let logMessage = utils.value(errorMessage, '', error, info); - rollbar[level](logMessage, error, data, callback); - } + reportWithContext(rollbar, this.reportContext, () => { + if (!errorMessage) { + rollbar[level](error, data, callback); + } else { + let logMessage = utils.value(errorMessage, '', error, info); + rollbar[level](logMessage, error, data, callback); + } + }); } resetError = () => { @@ -60,14 +64,23 @@ export class ErrorBoundary extends Component { const { hasError, error } = this.state; const { fallbackUI: FallbackUI, children } = this.props; + let content = null; if (!hasError) { - return children; - } - - if (!FallbackUI) { - return null; + content = children; + } else if (FallbackUI) { + content = ; } - return ; + // contextType is taken by the Provider's context. The value is read again + // whenever this renders, including the render after a child throws, which + // is the one React commits before calling componentDidCatch. + return ( + + {(reportContext) => { + this.reportContext = reportContext; + return content; + }} + + ); } } diff --git a/src/rollbar-context.js b/src/rollbar-context.js index b190649..bc13163 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -1,6 +1,6 @@ 'use client'; -import { Component } from 'react'; +import React, { Component, createContext } from 'react'; import PropTypes from 'prop-types'; import { Context, getRollbarFromContext } from './provider'; import { @@ -10,6 +10,11 @@ import { setRenderContext, } from './context-stack'; +// The nearest RollbarContext's order and context, for an ErrorBoundary inside +// it to report with; see reportWithContext in context-stack.js. +export const ReportContext = createContext(undefined); +ReportContext.displayName = 'RollbarReportContext'; + export class RollbarContext extends Component { static propTypes = { context: PropTypes.string.isRequired, @@ -27,6 +32,8 @@ export class RollbarContext extends Component { order = nextContextOrder(); // The context this component has set since mounting, if it's mounted. mountedContext = undefined; + // What ReportContext provides, kept until the context prop changes. + reportContext = undefined; changeContext = () => { this.mountedContext = this.props.context; @@ -42,8 +49,7 @@ export class RollbarContext extends Component { } componentDidUpdate(prevProps) { - const { onRender, context } = this.props; - if (!onRender || context !== prevProps.context) { + if (this.props.context !== prevProps.context) { this.changeContext(); } } @@ -65,6 +71,13 @@ export class RollbarContext extends Component { context, ); } - return this.props.children; + if (this.reportContext?.context !== context) { + this.reportContext = { order: this.order, context }; + } + return ( + + {this.props.children} + + ); } } diff --git a/src/tests/components/rollbar-context.server.test.tsx b/src/tests/components/rollbar-context.server.test.tsx index 2d62542..68b84f6 100644 --- a/src/tests/components/rollbar-context.server.test.tsx +++ b/src/tests/components/rollbar-context.server.test.tsx @@ -7,12 +7,19 @@ import Rollbar from 'rollbar'; import { Provider, useRollbarContext } from '../rollbar-react'; describe('useRollbarContext on the server', () => { + let consoleError: jest.SpyInstance; + + beforeEach(() => { + consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleError.mockRestore(); + }); + it.each([false, true])( 'renders without the useLayoutEffect warning (isLayout: %s)', (isLayout) => { - const consoleError = jest - .spyOn(console, 'error') - .mockImplementation(() => {}); const rollbar = new Rollbar({ accessToken: 'POST_SERVER_ITEM_TOKEN', enabled: false, @@ -28,7 +35,6 @@ describe('useRollbarContext on the server', () => { , ); expect(consoleError).not.toHaveBeenCalled(); - consoleError.mockRestore(); }, ); }); diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index 3116852..0e6ade8 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -1,4 +1,5 @@ -import React from 'react'; +import React, { useLayoutEffect } from 'react'; +import { format } from 'util'; import { renderToString } from 'react-dom/server'; import { render } from '@testing-library/react'; import Rollbar from 'rollbar'; @@ -30,7 +31,15 @@ describe('RollbarContext', () => { let consoleError: jest.SpyInstance; beforeEach(() => { - consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + const setupConsoleError = console.error; + consoleError = jest + .spyOn(console, 'error') + .mockImplementation((...args: unknown[]) => { + // Keep jest-setup.ts failing the test on a prop-type error. + if (/Failed prop type/.test(format(...args))) { + setupConsoleError(...args); + } + }); }); afterEach(() => { @@ -82,6 +91,32 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('root'); }); + it('does not re-apply an unchanged context when it re-renders', () => { + const rollbar = makeRollbar({ payload: { context: 'root' } }); + const ui = (text: string) => ( + + +
{text}
+
+
+ ); + + const { rerender } = render(ui('a')); + rollbar.configure({ payload: { context: 'elsewhere' } }); + rerender(ui('b')); + expect(contextOf(rollbar)).toBe('elsewhere'); + }); + + it('renders without children', () => { + const rollbar = makeRollbar(); + render( + + + , + ); + expect(contextOf(rollbar)).toBe('home'); + }); + // #88 describe('when a child throws while rendering', () => { const makeReporting = () => { @@ -113,6 +148,23 @@ describe('RollbarContext', () => { return reporting; }; + it('reports without touching the client outside a RollbarContext', () => { + const error = jest.fn(); + // Like jest.mock('rollbar'): no options, and no configure(). + const MockRollbar = function (this: { error: jest.Mock }) { + this.error = error; + } as unknown as typeof Rollbar; + + render( + + + + + , + ); + expect(error).toHaveBeenCalledTimes(1); + }); + it('reports with the previous context by default', () => { expect(renderThrowing(false).reported).toEqual(['root']); }); @@ -162,124 +214,190 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('app2'); }); - it('reports with this context from outside the ErrorBoundary, on first render and after', () => { - const { rollbar, reported } = makeReporting(); - const ui = (throws: boolean, boundaryKey: number) => ( - - - - - - - - ); - - const { rerender } = render(ui(true, 1)); - rerender(ui(false, 2)); - rerender(ui(true, 2)); - expect(reported).toEqual(['home', 'home']); - }); - - // React unmounts the old page before the new page's ErrorBoundary reports, - // in the same commit. - it('reports with the new page context when the old page unmounts in the same commit', async () => { - const { rollbar, reported } = makeReporting(); - const Page = ({ name, throws }: { name: string; throws: boolean }) => ( - - - - - - ); - const ui = (page: string) => ( - - - - ); - - const { rerender } = render(ui('home')); - await afterMicrotasks(); - expect(contextOf(rollbar)).toBe('home'); - - rerender(ui('about')); - expect(reported).toEqual(['about']); - await afterMicrotasks(); - expect(contextOf(rollbar)).toBe('about'); - }); - - // React runs passive effect cleanups after the commit in which the - // ErrorBoundary reports, so the old page's hook has to leave before then. - it('reports with the new page context when the old page used useRollbarContext', async () => { - const { rollbar, reported } = makeReporting(); - const HomePage = () => { - useRollbarContext('home#index'); - return null; - }; - const ui = (page: string) => ( - - - - {page === 'home' ? : } - - - - ); - - const { rerender } = render(ui('home')); - await afterMicrotasks(); - expect(contextOf(rollbar)).toBe('home#index'); - - rerender(ui('about')); - expect(reported).toEqual(['about']); - await afterMicrotasks(); - expect(contextOf(rollbar)).toBe('about'); - }); - - // An earlier sibling's componentDidUpdate runs before the ErrorBoundary - // reports, in the same commit. - it('reports with this context when a sibling context updates in the same commit', async () => { - const { rollbar, reported } = makeReporting(); - const ui = (showPage: boolean) => ( - - -
- - {showPage && ( - + // From here on the RollbarContext is outside the ErrorBoundary, which + // reports with its context with or without onRender. + describe.each([false, true])( + 'outside the ErrorBoundary (onRender: %s)', + (onRender) => { + it('reports with this context, on first render and after', () => { + const { rollbar, reported } = makeReporting(); + const ui = (throws: boolean, boundaryKey: number) => ( + + + + + + + + ); + + const { rerender } = render(ui(true, 1)); + rerender(ui(false, 2)); + rerender(ui(true, 2)); + expect(reported).toEqual(['home', 'home']); + }); + + // React unmounts the old page before the new page's ErrorBoundary reports, + // in the same commit. + it('reports with the new page context when the old page unmounts in the same commit', async () => { + const { rollbar, reported } = makeReporting(); + const Page = ({ + name, + throws, + }: { + name: string; + throws: boolean; + }) => ( + - + - )} - - ); - - const { rerender } = render(ui(false)); - rerender(ui(true)); - expect(reported).toEqual(['home']); - await afterMicrotasks(); - expect(contextOf(rollbar)).toBe('home'); - }); - - it('reports with the new context when the context prop changes', async () => { - const { rollbar, reported } = makeReporting(); - // React reuses the RollbarContext and mounts a new page inside it. - const ui = (page: string) => ( - - - - - - - - ); - - const { rerender } = render(ui('home')); - rerender(ui('about')); - expect(reported).toEqual(['about']); - await afterMicrotasks(); - expect(contextOf(rollbar)).toBe('about'); - }); + ); + const ui = (page: string) => ( + + + + ); + + const { rerender } = render(ui('home')); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home'); + + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('about'); + }); + + // React runs passive effect cleanups after the commit in which the + // ErrorBoundary reports, so the old page's hook has to leave before then. + it('reports with the new page context when the old page used useRollbarContext', async () => { + const { rollbar, reported } = makeReporting(); + const HomePage = () => { + useRollbarContext('home#index'); + return null; + }; + const ui = (page: string) => ( + + + + {page === 'home' ? : } + + + + ); + + const { rerender } = render(ui('home')); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home#index'); + + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('about'); + }); + + // An earlier sibling's componentDidUpdate runs before the ErrorBoundary + // reports, in the same commit. + it('reports with this context when a sibling context updates in the same commit', async () => { + const { rollbar, reported } = makeReporting(); + const ui = (showPage: boolean) => ( + + +
+ + {showPage && ( + + + + + + )} + + ); + + const { rerender } = render(ui(false)); + rerender(ui(true)); + expect(reported).toEqual(['home']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home'); + }); + + it('reports with the new context when the context prop changes', async () => { + const { rollbar, reported } = makeReporting(); + // React reuses the RollbarContext and mounts a new page inside it. + const ui = (page: string) => ( + + + + + + + + ); + + const { rerender } = render(ui('home')); + rerender(ui('about')); + expect(reported).toEqual(['about']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('about'); + }); + + it('keeps the context of a useRollbarContext between it and the ErrorBoundary', async () => { + const { rollbar, reported } = makeReporting(); + const Page = ({ throws }: { throws: boolean }) => { + useRollbarContext('home#index'); + return ( + + + + ); + }; + const ui = (throws: boolean) => ( + + + + + + ); + + const { rerender } = render(ui(false)); + await afterMicrotasks(); + rerender(ui(true)); + expect(reported).toEqual(['home#index']); + expect(contextOf(rollbar)).toBe('home#index'); + }); + + // React calls componentDidCatch before the RollbarContext mounts. + it('puts the previous context back after reporting', async () => { + const { rollbar, reported } = makeReporting(); + let afterReport: unknown; + const Probe = () => { + useLayoutEffect(() => { + afterReport = contextOf(rollbar); + }, []); + return null; + }; + + render( + + + + + + + + , + ); + expect(reported).toEqual(['home']); + // With onRender, the context set during render is still there. + expect(afterReport).toBe(onRender ? 'home' : 'root'); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('home'); + }); + }, + ); it('leaves nothing behind when the ErrorBoundary replaces an updated onRender context', async () => { const { rollbar, reported } = makeReporting(); diff --git a/src/tests/jest-setup.ts b/src/tests/jest-setup.ts index 7de749a..5c875b5 100644 --- a/src/tests/jest-setup.ts +++ b/src/tests/jest-setup.ts @@ -1,9 +1,12 @@ import '@testing-library/jest-dom'; import 'regenerator-runtime'; +import { format } from 'util'; -// Let propType errors cause test failure. +// Let propType errors cause test failure. React passes the message as a +// format string ('Failed %s type: %s%s', 'prop', ...), so format it first. const originalConsoleError = console.error; -console.error = (message: string) => { +console.error = (...args: unknown[]) => { + const message = format(...args); if (/(Failed prop type)/.test(message)) throw new Error(message); - originalConsoleError(message); + originalConsoleError(...args); }; From bc8f28dcd9f34e8232fbb3d180dabbe951db982b Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 9 Oct 2026 22:47:38 +0000 Subject: [PATCH 11/13] Only let contexts around the ErrorBoundary override its RollbarContext reportWithContext kept the client's context whenever any stack entry ranked after the boundary's RollbarContext. Order is only creation order, so an unrelated sibling RollbarContext or hook created later (a footer, a sidebar) also won, and the result depended on mount history. RollbarContext and ErrorBoundary now provide a scope path, the orders of the RollbarContexts and ErrorBoundaries around a component, through an internal ScopeContext, and every stack entry records its path. The boundary only gives way to an entry inside its RollbarContext that is either inside the boundary (an onRender render React threw away) or a useRollbarContext that rendered before the boundary. A hook doesn't provide a scope, so one in an earlier sibling of the boundary inside the same RollbarContext still counts; the README says so. Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +- src/context-stack.js | 80 ++++++++++++---- src/error-boundary.js | 32 +++++-- src/hooks/use-rollbar-context.js | 16 +++- src/rollbar-context.js | 29 +++++- src/tests/components/rollbar-context.test.tsx | 95 +++++++++++++++++++ 6 files changed, 224 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index bd854ba..55abb39 100644 --- a/README.md +++ b/README.md @@ -398,7 +398,10 @@ Put the `RollbarContext` outside the `ErrorBoundary`: The `ErrorBoundary` reports an error with the context of the nearest `RollbarContext` around it. That includes an error thrown while they're first rendering, before the `RollbarContext` has mounted, and an error thrown after a change to the `context` prop, before it has been applied. A `useRollbarContext` hook between the two that has set its -context still takes precedence, as the innermost one. +context still takes precedence, as the innermost one. A `RollbarContext` or hook elsewhere in the tree doesn't, even if +it was mounted later and set the client's context, like a sibling of the `RollbarContext`. A hook doesn't mark where it +is in the tree, so one in a component that renders before the `ErrorBoundary` inside the same `RollbarContext`, like a +sidebar, counts as between them. Outside, because when the `ErrorBoundary` catches an error, React removes everything inside it before the error is reported. A `RollbarContext` inside it has already been removed by then. diff --git a/src/context-stack.js b/src/context-stack.js index c4a2929..9076394 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -25,6 +25,9 @@ function getStack(rollbar) { contexts: new Map(), // From setRenderContext: set during render, until React has committed. rendering: new Map(), + // The scope path (see ScopeContext in rollbar-context.js) of the + // component each order belongs to, for reportWithContext. + paths: new Map(), }; stacks.set(rollbar, stack); } @@ -35,15 +38,24 @@ function innermostOrder({ contexts, rendering }) { return Math.max(...contexts.keys(), ...rendering.keys()); } +// A component that's rendering with a new context has the old one in +// `contexts` until it updates. +function contextAt({ contexts, rendering }, order) { + return rendering.has(order) + ? rendering.get(order).context + : contexts.get(order); +} + +function forgetPath(stack, order) { + if (!stack.contexts.has(order) && !stack.rendering.has(order)) { + stack.paths.delete(order); + } +} + function applyStack(rollbar, stack) { const { contexts, rendering } = stack; if (contexts.size || rendering.size) { - const innermost = innermostOrder(stack); - // A component that's rendering with a new context has the old one in - // `contexts` until it updates. - const context = rendering.has(innermost) - ? rendering.get(innermost).context - : contexts.get(innermost); + const context = contextAt(stack, innermostOrder(stack)); rollbar.configure({ payload: { context } }); return; } @@ -59,17 +71,20 @@ function applyStack(rollbar, stack) { // Adds the context for `order`, or applies its new value if it's already // there. It replaces any context setRenderContext set for `order`: the -// component has mounted or updated, so that render was committed. -export function setContext(rollbar, order, context) { +// component has mounted or updated, so that render was committed. `path` is +// the component's scope path. +export function setContext(rollbar, order, context, path) { const stack = getStack(rollbar); stack.rendering.delete(order); stack.contexts.set(order, context); + stack.paths.set(order, path); applyStack(rollbar, stack); } export function removeContext(rollbar, order) { const stack = stacks.get(rollbar); if (stack?.contexts.delete(order)) { + forgetPath(stack, order); applyStack(rollbar, stack); } } @@ -90,15 +105,17 @@ export function removeContext(rollbar, order) { // loaded. Then the microtask runs first, and the children mount with the // previous context, as without onRender. ErrorBoundary doesn't rely on this; // see reportWithContext. -export function setRenderContext(rollbar, order, context) { +export function setRenderContext(rollbar, order, context, path) { const stack = getStack(rollbar); const entry = { context }; stack.rendering.set(order, entry); + stack.paths.set(order, path); applyStack(rollbar, stack); Promise.resolve().then(() => { // Unless a later render replaced it; its own microtask removes that. if (stack.rendering.get(order) === entry) { stack.rendering.delete(order); + forgetPath(stack, order); applyStack(rollbar, stack); } }); @@ -112,27 +129,54 @@ export function setRenderContext(rollbar, order, context) { // client still has the previous context. Under onRender, React may also have // committed after the microtask in setRenderContext. // +// An entry inside that RollbarContext takes precedence, the innermost one, if +// it's one of: +// - inside the ErrorBoundary. React has removed what was mounted there before +// the ErrorBoundary reports, so that's an onRender context whose render +// React threw away, around the child that threw. +// - a useRollbarContext that rendered before the ErrorBoundary: one between +// them. A hook doesn't provide a scope, so one in an earlier sibling of the +// ErrorBoundary counts too, as it does for the client's context. +// Other entries can rank after the RollbarContext without being around the +// ErrorBoundary, like a sibling RollbarContext or hook created later, so the +// scope paths decide. A RollbarContext between them would be the nearest one. +// // rollbar.js has no per-item context, so the context is applied around the // report: rollbar.js takes the options an item is sent with when it's logged, -// and configure() replaces them rather than changing them. An entry that ranks -// after the RollbarContext, like a useRollbarContext between it and the -// ErrorBoundary, keeps the context it set. -export function reportWithContext(rollbar, reportContext, report) { +// and configure() replaces them rather than changing them. +export function reportWithContext( + rollbar, + reportContext, + boundaryPath, + report, +) { // Without a RollbarContext, the client isn't touched, as before. if (!reportContext) { report(); return; } + const scope = reportContext.order; + const boundary = boundaryPath[boundaryPath.length - 1]; + let { context } = reportContext; + let innermost = scope; const stack = stacks.get(rollbar); + stack?.paths.forEach((path, order) => { + if (order <= innermost || !path.includes(scope)) { + return; + } + // A RollbarContext's path ends with its own order, a hook's doesn't. + const isHook = path[path.length - 1] !== order; + if (path.includes(boundary) || (isHook && order < boundary)) { + innermost = order; + context = contextAt(stack, order); + } + }); const previous = rollbar.options.payload?.context; - if ( - (stack && innermostOrder(stack) > reportContext.order) || - reportContext.context === previous - ) { + if (context === previous) { report(); return; } - rollbar.configure({ payload: { context: reportContext.context } }); + rollbar.configure({ payload: { context } }); try { report(); } finally { diff --git a/src/error-boundary.js b/src/error-boundary.js index 2df31fc..7d0413d 100644 --- a/src/error-boundary.js +++ b/src/error-boundary.js @@ -5,8 +5,8 @@ import PropTypes from 'prop-types'; import invariant from 'tiny-invariant'; import { LEVEL_ERROR } from './constants'; import { Context, getRollbarFromContext } from './provider'; -import { ReportContext } from './rollbar-context'; -import { reportWithContext } from './context-stack'; +import { ReportContext, ScopeContext } from './rollbar-context'; +import { nextContextOrder, reportWithContext } from './context-stack'; import * as utils from './utils'; const INITIAL_ERROR_STATE = { hasError: false, error: null }; @@ -36,6 +36,11 @@ export class ErrorBoundary extends Component { this.state = { ...INITIAL_ERROR_STATE }; } + // Ranks this among the contexts around and inside it, and what ScopeContext + // provides; see reportWithContext. + order = nextContextOrder(); + path = undefined; + static getDerivedStateFromError(error) { return { hasError: true, error }; } @@ -46,7 +51,7 @@ export class ErrorBoundary extends Component { const data = { ...info, ...custom }; const level = utils.value(targetLevel, LEVEL_ERROR, error, info); const rollbar = getRollbarFromContext(this.context); - reportWithContext(rollbar, this.reportContext, () => { + reportWithContext(rollbar, this.reportContext, this.path, () => { if (!errorMessage) { rollbar[level](error, data, callback); } else { @@ -75,12 +80,23 @@ export class ErrorBoundary extends Component { // whenever this renders, including the render after a child throws, which // is the one React commits before calling componentDidCatch. return ( - - {(reportContext) => { - this.reportContext = reportContext; - return content; + + {(parentPath) => { + this.path ??= [...parentPath, this.order]; + return ( + + {(reportContext) => { + this.reportContext = reportContext; + return ( + + {content} + + ); + }} + + ); }} - + ); } } diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index b6c94e1..a1528af 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -1,9 +1,16 @@ 'use client'; import invariant from 'tiny-invariant'; -import { useEffect, useLayoutEffect, useRef, useState } from 'react'; +import { + useContext, + useEffect, + useLayoutEffect, + useRef, + useState, +} from 'react'; import { useRollbar } from './use-rollbar'; import { nextContextOrder, removeContext, setContext } from '../context-stack'; +import { ScopeContext } from '../rollbar-context'; // Before React 19, useLayoutEffect warns when rendering on the server, where // no effects run anyway. @@ -25,6 +32,9 @@ export function useRollbarContext(ctx = '', isLayout = false) { const rollbar = useRollbar(); // Where this component sits among nested contexts; see context-stack.js. const [order] = useState(nextContextOrder); + // The RollbarContexts and ErrorBoundaries around this hook; see + // reportWithContext in context-stack.js. + const path = useContext(ScopeContext); // The context this hook has set, once it has. const appliedRef = useRef(); // Removed in a layout effect cleanup, whatever `isLayout` is. When React @@ -35,7 +45,7 @@ export function useRollbarContext(ctx = '', isLayout = false) { // React 18 and later also run layout cleanups when Suspense hides a tree // that was showing, and run only layout effects again when it shows it. if (appliedRef.current !== undefined) { - setContext(rollbar, order, appliedRef.current); + setContext(rollbar, order, appliedRef.current, path); } return () => removeContext(rollbar, order); }, []); @@ -43,7 +53,7 @@ export function useRollbarContext(ctx = '', isLayout = false) { isLayout, () => { appliedRef.current = ctx; - setContext(rollbar, order, ctx); + setContext(rollbar, order, ctx, path); }, [ctx], ); diff --git a/src/rollbar-context.js b/src/rollbar-context.js index bc13163..9886931 100644 --- a/src/rollbar-context.js +++ b/src/rollbar-context.js @@ -15,6 +15,13 @@ import { export const ReportContext = createContext(undefined); ReportContext.displayName = 'RollbarReportContext'; +// The scope path: the orders of the RollbarContexts and ErrorBoundaries +// around a component, outermost first, for reportWithContext. Each one +// provides the path to itself, which never changes while it's mounted, so the +// components that use useRollbarContext don't re-render when it changes. +export const ScopeContext = createContext([]); +ScopeContext.displayName = 'RollbarScopeContext'; + export class RollbarContext extends Component { static propTypes = { context: PropTypes.string.isRequired, @@ -34,6 +41,8 @@ export class RollbarContext extends Component { mountedContext = undefined; // What ReportContext provides, kept until the context prop changes. reportContext = undefined; + // What ScopeContext provides, set on the first render. + path = undefined; changeContext = () => { this.mountedContext = this.props.context; @@ -41,6 +50,7 @@ export class RollbarContext extends Component { getRollbarFromContext(this.context), this.order, this.props.context, + this.path, ); }; @@ -60,7 +70,17 @@ export class RollbarContext extends Component { } render() { + // contextType is taken by the Provider's context. + return ( + + {(parentPath) => this.renderInScope(parentPath)} + + ); + } + + renderInScope(parentPath) { const { onRender, context } = this.props; + this.path ??= [...parentPath, this.order]; if (onRender && context !== this.mountedContext) { // Before the children render, on the first render and when the context // prop changes, so that errors they throw are reported with this @@ -69,15 +89,18 @@ export class RollbarContext extends Component { getRollbarFromContext(this.context), this.order, context, + this.path, ); } if (this.reportContext?.context !== context) { this.reportContext = { order: this.order, context }; } return ( - - {this.props.children} - + + + {this.props.children} + + ); } } diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index 0e6ade8..59d2bcd 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -369,6 +369,101 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('home#index'); }); + // They rank after it too, but aren't between it and the ErrorBoundary. + it('ignores a later sibling RollbarContext', async () => { + const { rollbar, reported } = makeReporting(); + const ui = (throws: boolean) => ( + + + + + + + + + ); + + const { rerender } = render(ui(false)); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('footer'); + rerender(ui(true)); + expect(reported).toEqual(['home']); + expect(contextOf(rollbar)).toBe('footer'); + }); + + it('ignores a useRollbarContext after the ErrorBoundary or outside the RollbarContext', async () => { + const { rollbar, reported } = makeReporting(); + const Hook = ({ ctx }: { ctx: string }) => { + useRollbarContext(ctx); + return null; + }; + const ui = (throws: boolean) => ( + + + + + + + + + + ); + + const { rerender } = render(ui(false)); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('footer'); + rerender(ui(true)); + expect(reported).toEqual(['home']); + expect(contextOf(rollbar)).toBe('footer'); + }); + + // With onRender, it's still rendering when the ErrorBoundary reports. + it('ignores a later sibling onRender RollbarContext in the same commit', async () => { + const { rollbar, reported } = makeReporting(); + render( + + + + + + + + , + ); + expect(reported).toEqual(['home']); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('sidebar'); + }); + + it('keeps the innermost of several useRollbarContext hooks between it and the ErrorBoundary', async () => { + const { rollbar, reported } = makeReporting(); + const Inner = ({ throws }: { throws: boolean }) => { + useRollbarContext('home#show'); + return ( + + + + ); + }; + const Outer = ({ throws }: { throws: boolean }) => { + useRollbarContext('home#index'); + return ; + }; + const ui = (throws: boolean) => ( + + + + + + + ); + + const { rerender } = render(ui(false)); + await afterMicrotasks(); + rerender(ui(true)); + expect(reported).toEqual(['home#show']); + }); + // React calls componentDidCatch before the RollbarContext mounts. it('puts the previous context back after reporting', async () => { const { rollbar, reported } = makeReporting(); From e9cae5e47d0ed2febaa944bea79b9f4a162baf16 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 9 Oct 2026 23:39:17 +0000 Subject: [PATCH 12/13] Don't let a hook in a sibling's scope override the ErrorBoundary's context reportWithContext treated any useRollbarContext inside the boundary's RollbarContext that took an earlier order number as between them. A hook records its scope path, so one inside a sibling ErrorBoundary or RollbarContext can be told apart: only count a hook whose scopes are all around the ErrorBoundary. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 ++-- src/context-stack.js | 15 ++++++--- src/tests/components/rollbar-context.test.tsx | 31 +++++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 55abb39..fd15769 100644 --- a/README.md +++ b/README.md @@ -399,9 +399,9 @@ The `ErrorBoundary` reports an error with the context of the nearest `RollbarCon error thrown while they're first rendering, before the `RollbarContext` has mounted, and an error thrown after a change to the `context` prop, before it has been applied. A `useRollbarContext` hook between the two that has set its context still takes precedence, as the innermost one. A `RollbarContext` or hook elsewhere in the tree doesn't, even if -it was mounted later and set the client's context, like a sibling of the `RollbarContext`. A hook doesn't mark where it -is in the tree, so one in a component that renders before the `ErrorBoundary` inside the same `RollbarContext`, like a -sidebar, counts as between them. +it was mounted later and set the client's context, like a sibling of the `RollbarContext`. A hook doesn't provide a +scope of its own, so one in a component that renders before the `ErrorBoundary` inside the same `RollbarContext`, like a +sidebar, counts as between them. One inside another `ErrorBoundary` or `RollbarContext` there doesn't. Outside, because when the `ErrorBoundary` catches an error, React removes everything inside it before the error is reported. A `RollbarContext` inside it has already been removed by then. diff --git a/src/context-stack.js b/src/context-stack.js index 9076394..aa67293 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -134,9 +134,11 @@ export function setRenderContext(rollbar, order, context, path) { // - inside the ErrorBoundary. React has removed what was mounted there before // the ErrorBoundary reports, so that's an onRender context whose render // React threw away, around the child that threw. -// - a useRollbarContext that rendered before the ErrorBoundary: one between -// them. A hook doesn't provide a scope, so one in an earlier sibling of the -// ErrorBoundary counts too, as it does for the client's context. +// - a useRollbarContext that rendered before the ErrorBoundary, and whose +// scopes are all around it: one between them. A hook doesn't provide a +// scope, so one in an earlier sibling of the ErrorBoundary counts too, as it +// does for the client's context, unless it's inside a sibling ErrorBoundary +// or RollbarContext. // Other entries can rank after the RollbarContext without being around the // ErrorBoundary, like a sibling RollbarContext or hook created later, so the // scope paths decide. A RollbarContext between them would be the nearest one. @@ -166,7 +168,12 @@ export function reportWithContext( } // A RollbarContext's path ends with its own order, a hook's doesn't. const isHook = path[path.length - 1] !== order; - if (path.includes(boundary) || (isHook && order < boundary)) { + // Every scope around the hook is around the ErrorBoundary too. + const aroundBoundary = path.every((o, i) => o === boundaryPath[i]); + if ( + path.includes(boundary) || + (isHook && aroundBoundary && order < boundary) + ) { innermost = order; context = contextAt(stack, order); } diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index 59d2bcd..6820daf 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -417,6 +417,37 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('footer'); }); + // They render before it, but in a sibling's scope. + it('ignores a useRollbarContext inside a sibling ErrorBoundary or RollbarContext', async () => { + const { rollbar, reported } = makeReporting(); + const Hook = ({ ctx }: { ctx: string }) => { + useRollbarContext(ctx); + return null; + }; + const ui = (throws: boolean) => ( + + + + + + + + + + + + + + ); + + const { rerender } = render(ui(false)); + await afterMicrotasks(); + expect(contextOf(rollbar)).toBe('nav#menu'); + rerender(ui(true)); + expect(reported).toEqual(['dashboard']); + expect(contextOf(rollbar)).toBe('nav#menu'); + }); + // With onRender, it's still rendering when the ErrorBoundary reports. it('ignores a later sibling onRender RollbarContext in the same commit', async () => { const { rollbar, reported } = makeReporting(); From 7d23894022c3912554c71d918fb97e876ce3c5b5 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Sat, 10 Oct 2026 02:37:51 +0000 Subject: [PATCH 13/13] Report with the context of a hook that rendered with the error Removing a useRollbarContext entry in a layout cleanup keeps the old page's hook from tagging the next page's error, but React also runs that cleanup before componentDidCatch when the hook's own component throws after mounting, so the error lost the hook's context. The TypeScript example (ErrorBoundary around a page that calls the hook and throws on a state update) reported '' instead of '/Home'. The hook now records the context and scope path it renders with in a registry that only reportWithContext reads; it never changes the client's context. A layout effect clears it on commit, and a microtask clears renders React throws away. The ErrorBoundary copies it while it renders after catching, so a commit React 19 holds back (a precedence stylesheet) still has it. A hook in a component that rendered with the error (it threw, or a child rendering with it did) takes precedence like any other entry inside the boundary, also without a RollbarContext. One that didn't render with it, like the previous page or a child throwing on its own update, still doesn't. A sibling hook no longer has to have an earlier order than the ErrorBoundary to count as between it and its RollbarContext. Order is creation order, so a keyed ErrorBoundary that remounts ranked after a footer hook it used to rank before. A hook doesn't provide a scope, so any hook whose scopes are all around the boundary counts, before or after it, as it does for the client's context. Co-Authored-By: Claude Opus 5.5 --- README.md | 22 ++- src/context-stack.js | 87 ++++++--- src/error-boundary.js | 37 +++- src/hooks/use-rollbar-context.js | 16 +- src/tests/components/rollbar-context.test.tsx | 166 +++++++++++++++++- 5 files changed, 284 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index fd15769..ed1add6 100644 --- a/README.md +++ b/README.md @@ -397,11 +397,12 @@ Put the `RollbarContext` outside the `ErrorBoundary`: The `ErrorBoundary` reports an error with the context of the nearest `RollbarContext` around it. That includes an error thrown while they're first rendering, before the `RollbarContext` has mounted, and an error thrown after a -change to the `context` prop, before it has been applied. A `useRollbarContext` hook between the two that has set its -context still takes precedence, as the innermost one. A `RollbarContext` or hook elsewhere in the tree doesn't, even if -it was mounted later and set the client's context, like a sibling of the `RollbarContext`. A hook doesn't provide a -scope of its own, so one in a component that renders before the `ErrorBoundary` inside the same `RollbarContext`, like a -sidebar, counts as between them. One inside another `ErrorBoundary` or `RollbarContext` there doesn't. +change to the `context` prop, before it has been applied. A `useRollbarContext` hook between the two still takes +precedence, as the innermost one, and so does one inside the `ErrorBoundary` in a component that rendered with the +error; see [the hook](#userollbarcontext-hook). A `RollbarContext` or hook elsewhere in the tree doesn't, even if it +was mounted later and set the client's context, like a sibling of the `RollbarContext`. A hook doesn't provide a scope +of its own, so one in another component inside the same `RollbarContext`, like a sidebar or a footer, counts as between +them, as it does for the client's context. One inside another `ErrorBoundary` or `RollbarContext` there doesn't. Outside, because when the `ErrorBoundary` catches an error, React removes everything inside it before the error is reported. A `RollbarContext` inside it has already been removed by then. @@ -611,10 +612,13 @@ As an alternative to the [`RollbarContext`] component, you can use the `useRollb to set the `context` in the [Rollbar.js] client provided by the [`Provider`] above in the React Tree. The hook sets the context in an effect, so like `RollbarContext` without `onRender`, it doesn't apply to anything sent -while the component and its children are first rendering and mounting, and an `ErrorBoundary` inside the component -can't report with it then either. It's removed as soon as React removes the component, though, so in the -[`ErrorBoundary` pattern](#using-with-errorboundary) a page that used it doesn't set the context of an error the next -page throws. +while the component and its children are first rendering and mounting. An `ErrorBoundary` around or inside the +component does report with it, including then, if the component rendered with the error: it threw, or a child that +rendered with it did. React removes the component before an `ErrorBoundary` around it reports, and the hook's context +with it, so an error the component didn't render with, like one a child throws on its own state update or in an +effect, gets the context around the component. That's also why, in the +[`ErrorBoundary` pattern](#using-with-errorboundary), a page that used the hook doesn't set the context of an error the +next page throws. Here's an example of using it in several components: diff --git a/src/context-stack.js b/src/context-stack.js index aa67293..97a7f3f 100644 --- a/src/context-stack.js +++ b/src/context-stack.js @@ -121,6 +121,40 @@ export function setRenderContext(rollbar, order, context, path) { }); } +// From useRollbarContext: the context and scope path each hook rendered with, +// per Rollbar client, from its render until it commits. When React removes a +// component, it removes the hook's entry from the stack before an +// ErrorBoundary reports, so this is how the ErrorBoundary knows the context of +// a hook that rendered with the error, like one in the component that threw. +// It never changes the client's context. If React throws the render away, a +// microtask removes it; the ErrorBoundary takes a copy while it renders after +// catching the error, so when React commits doesn't matter. +const hookRenders = new WeakMap(); + +export function setHookRender(rollbar, order, context, path) { + let rendered = hookRenders.get(rollbar); + if (!rendered) { + rendered = new Map(); + hookRenders.set(rollbar, rendered); + Promise.resolve().then(() => { + if (hookRenders.get(rollbar) === rendered) { + hookRenders.delete(rollbar); + } + }); + } + rendered.set(order, { context, path }); +} + +export function clearHookRender(rollbar, order) { + hookRenders.get(rollbar)?.delete(order); +} + +// For ErrorBoundary, while it renders after catching an error. +export function getHookRenders(rollbar) { + const rendered = hookRenders.get(rollbar); + return rendered && new Map(rendered); +} + // For ErrorBoundary: calls `report` with the context of the nearest // RollbarContext around the ErrorBoundary, `reportContext`, which React // resolved while rendering it. That RollbarContext sets its context when it @@ -133,15 +167,23 @@ export function setRenderContext(rollbar, order, context, path) { // it's one of: // - inside the ErrorBoundary. React has removed what was mounted there before // the ErrorBoundary reports, so that's an onRender context whose render -// React threw away, around the child that threw. -// - a useRollbarContext that rendered before the ErrorBoundary, and whose -// scopes are all around it: one between them. A hook doesn't provide a -// scope, so one in an earlier sibling of the ErrorBoundary counts too, as it -// does for the client's context, unless it's inside a sibling ErrorBoundary -// or RollbarContext. +// React threw away, around the child that threw, or a hook in +// `renderedHooks`, the copy of setHookRender's entries the ErrorBoundary +// took when it rendered. A hook in a component that didn't render with the +// error, like the previous page, has been removed. +// - a useRollbarContext whose scopes are all around the ErrorBoundary: one +// between them. A hook doesn't provide a scope, so one in a sibling of the +// ErrorBoundary counts too, before or after it, as it does for the client's +// context, unless it's inside a sibling ErrorBoundary or RollbarContext. // Other entries can rank after the RollbarContext without being around the // ErrorBoundary, like a sibling RollbarContext or hook created later, so the // scope paths decide. A RollbarContext between them would be the nearest one. +// A hook's entry in `renderedHooks` replaces its entry in the stack: it's the +// context it has rendered with, which it sets once it commits. +// +// Without a RollbarContext, the same entries take precedence over the +// client's context, and if there aren't any, the client isn't touched, as +// before. // // rollbar.js has no per-item context, so the context is applied around the // report: rollbar.js takes the options an item is sent with when it's logged, @@ -150,34 +192,39 @@ export function reportWithContext( rollbar, reportContext, boundaryPath, + renderedHooks, report, ) { - // Without a RollbarContext, the client isn't touched, as before. - if (!reportContext) { - report(); - return; - } - const scope = reportContext.order; + const scope = reportContext?.order; const boundary = boundaryPath[boundaryPath.length - 1]; - let { context } = reportContext; - let innermost = scope; + let context = reportContext?.context; + // Orders start at 1. + let innermost = scope ?? 0; + const entries = new Map(); const stack = stacks.get(rollbar); stack?.paths.forEach((path, order) => { - if (order <= innermost || !path.includes(scope)) { + entries.set(order, { path, context: contextAt(stack, order) }); + }); + renderedHooks?.forEach((entry, order) => { + entries.set(order, entry); + }); + entries.forEach(({ path, context: entryContext }, order) => { + if (order <= innermost || (scope && !path.includes(scope))) { return; } // A RollbarContext's path ends with its own order, a hook's doesn't. const isHook = path[path.length - 1] !== order; // Every scope around the hook is around the ErrorBoundary too. const aroundBoundary = path.every((o, i) => o === boundaryPath[i]); - if ( - path.includes(boundary) || - (isHook && aroundBoundary && order < boundary) - ) { + if (path.includes(boundary) || (isHook && aroundBoundary)) { innermost = order; - context = contextAt(stack, order); + context = entryContext; } }); + if (!innermost) { + report(); + return; + } const previous = rollbar.options.payload?.context; if (context === previous) { report(); diff --git a/src/error-boundary.js b/src/error-boundary.js index 7d0413d..3845a98 100644 --- a/src/error-boundary.js +++ b/src/error-boundary.js @@ -6,7 +6,11 @@ import invariant from 'tiny-invariant'; import { LEVEL_ERROR } from './constants'; import { Context, getRollbarFromContext } from './provider'; import { ReportContext, ScopeContext } from './rollbar-context'; -import { nextContextOrder, reportWithContext } from './context-stack'; +import { + getHookRenders, + nextContextOrder, + reportWithContext, +} from './context-stack'; import * as utils from './utils'; const INITIAL_ERROR_STATE = { hasError: false, error: null }; @@ -40,6 +44,9 @@ export class ErrorBoundary extends Component { // provides; see reportWithContext. order = nextContextOrder(); path = undefined; + // The useRollbarContext hooks that rendered with the error; see + // setHookRender. + renderedHooks = undefined; static getDerivedStateFromError(error) { return { hasError: true, error }; @@ -51,14 +58,20 @@ export class ErrorBoundary extends Component { const data = { ...info, ...custom }; const level = utils.value(targetLevel, LEVEL_ERROR, error, info); const rollbar = getRollbarFromContext(this.context); - reportWithContext(rollbar, this.reportContext, this.path, () => { - if (!errorMessage) { - rollbar[level](error, data, callback); - } else { - let logMessage = utils.value(errorMessage, '', error, info); - rollbar[level](logMessage, error, data, callback); - } - }); + reportWithContext( + rollbar, + this.reportContext, + this.path, + this.renderedHooks, + () => { + if (!errorMessage) { + rollbar[level](error, data, callback); + } else { + let logMessage = utils.value(errorMessage, '', error, info); + rollbar[level](logMessage, error, data, callback); + } + }, + ); } resetError = () => { @@ -69,6 +82,12 @@ export class ErrorBoundary extends Component { const { hasError, error } = this.state; const { fallbackUI: FallbackUI, children } = this.props; + // React renders this right after the child that threw, before it commits, + // which may not be in the same task. + this.renderedHooks = hasError + ? getHookRenders(getRollbarFromContext(this.context)) + : undefined; + let content = null; if (!hasError) { content = children; diff --git a/src/hooks/use-rollbar-context.js b/src/hooks/use-rollbar-context.js index a1528af..73d5b99 100644 --- a/src/hooks/use-rollbar-context.js +++ b/src/hooks/use-rollbar-context.js @@ -9,7 +9,13 @@ import { useState, } from 'react'; import { useRollbar } from './use-rollbar'; -import { nextContextOrder, removeContext, setContext } from '../context-stack'; +import { + clearHookRender, + nextContextOrder, + removeContext, + setContext, + setHookRender, +} from '../context-stack'; import { ScopeContext } from '../rollbar-context'; // Before React 19, useLayoutEffect warns when rendering on the server, where @@ -37,10 +43,18 @@ export function useRollbarContext(ctx = '', isLayout = false) { const path = useContext(ScopeContext); // The context this hook has set, once it has. const appliedRef = useRef(); + // For an ErrorBoundary that catches an error thrown while this renders, by + // this component or one inside it: React removes the component, and the + // entry below, before the ErrorBoundary reports. It doesn't change the + // client's context; see setHookRender in context-stack.js. + setHookRender(rollbar, order, ctx, path); + useClientLayoutEffect(() => clearHookRender(rollbar, order)); // Removed in a layout effect cleanup, whatever `isLayout` is. When React // removes a component, it runs its layout cleanups during the commit, but // its passive ones only after it, so after an ErrorBoundary elsewhere in the // tree has reported an error thrown in that commit, like the next page's. + // The ErrorBoundary gets the context of a hook that rendered with the error + // from setHookRender above. useClientLayoutEffect(() => { // React 18 and later also run layout cleanups when Suspense hides a tree // that was showing, and run only layout effects again when it shows it. diff --git a/src/tests/components/rollbar-context.test.tsx b/src/tests/components/rollbar-context.test.tsx index 6820daf..95251ba 100644 --- a/src/tests/components/rollbar-context.test.tsx +++ b/src/tests/components/rollbar-context.test.tsx @@ -1,7 +1,7 @@ -import React, { useLayoutEffect } from 'react'; +import React, { useLayoutEffect, useState } from 'react'; import { format } from 'util'; import { renderToString } from 'react-dom/server'; -import { render } from '@testing-library/react'; +import { act, render } from '@testing-library/react'; import Rollbar from 'rollbar'; import { ErrorBoundary, @@ -165,6 +165,51 @@ describe('RollbarContext', () => { expect(error).toHaveBeenCalledTimes(1); }); + // React removes the component, and the hook's entry, before the + // ErrorBoundary reports. + it('reports with the context of a useRollbarContext in the component that throws on an update', () => { + const { rollbar, reported } = makeReporting(); + const Page = ({ throws }: { throws: boolean }) => { + useRollbarContext('home#index'); + if (throws) { + throw new Error('update error'); + } + return null; + }; + const ui = (throws: boolean) => ( + + + + + + ); + + const { rerender } = render(ui(false)); + expect(contextOf(rollbar)).toBe('home#index'); + rerender(ui(true)); + expect(reported).toEqual(['home#index']); + expect(contextOf(rollbar)).toBe('root'); + }); + + it('reports with the context of a useRollbarContext around it on first render', () => { + const { rollbar, reported } = makeReporting(); + const Page = () => { + useRollbarContext('home#index'); + return ( + + + + ); + }; + render( + + + , + ); + expect(reported).toEqual(['home#index']); + expect(contextOf(rollbar)).toBe('home#index'); + }); + it('reports with the previous context by default', () => { expect(renderThrowing(false).reported).toEqual(['root']); }); @@ -391,7 +436,10 @@ describe('RollbarContext', () => { expect(contextOf(rollbar)).toBe('footer'); }); - it('ignores a useRollbarContext after the ErrorBoundary or outside the RollbarContext', async () => { + // A hook doesn't provide a scope, so where it renders among the + // ErrorBoundary's siblings isn't known, and it counts as between them + // like it does for the client's context. + it('keeps the context of a useRollbarContext after the ErrorBoundary, but not outside the RollbarContext', async () => { const { rollbar, reported } = makeReporting(); const Hook = ({ ctx }: { ctx: string }) => { useRollbarContext(ctx); @@ -403,7 +451,7 @@ describe('RollbarContext', () => { - + @@ -413,10 +461,118 @@ describe('RollbarContext', () => { await afterMicrotasks(); expect(contextOf(rollbar)).toBe('footer'); rerender(ui(true)); - expect(reported).toEqual(['home']); + expect(reported).toEqual(['home#footer']); expect(contextOf(rollbar)).toBe('footer'); }); + // A new ErrorBoundary takes a new order, after the hook's. + it('treats a sibling useRollbarContext the same after the ErrorBoundary remounts', async () => { + const { rollbar, reported } = makeReporting(); + const Footer = () => { + useRollbarContext('app#footer'); + return null; + }; + const ui = (pathname: string, throws: boolean) => ( + + + + + +