From 0407cc324ae943153cbc452a96b19239e17bf818 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 25 Sep 2026 04:52:52 +0000 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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 ?? '' } }); }