Skip to content

Commit bc3d092

Browse files
committed
Preserve stack trace of errors inside store subscription callback
Ported changes from react-redux-hooks-poc Note: the "transient errors" test seems flawed atm.
1 parent 9c7fc93 commit bc3d092

File tree

2 files changed

+65
-3
lines changed

2 files changed

+65
-3
lines changed

src/hooks/useSelector.js

+23-2
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,33 @@ export function useSelector(selector) {
4646
contextSub
4747
])
4848

49+
const latestSubscriptionCallbackError = useRef()
4950
const latestSelector = useRef(selector)
50-
const selectedState = selector(store.getState())
51+
52+
let selectedState = undefined
53+
54+
try {
55+
selectedState = latestSelector.current(store.getState())
56+
} catch (err) {
57+
let errorMessage = `An error occured while selecting the store state: ${
58+
err.message
59+
}.`
60+
61+
if (latestSubscriptionCallbackError.current) {
62+
errorMessage += `\nThe error may be correlated with this previous error:\n${
63+
latestSubscriptionCallbackError.current.stack
64+
}\n\nOriginal stack trace:`
65+
}
66+
67+
throw new Error(errorMessage)
68+
}
69+
5170
const latestSelectedState = useRef(selectedState)
5271

5372
useIsomorphicLayoutEffect(() => {
5473
latestSelector.current = selector
5574
latestSelectedState.current = selectedState
75+
latestSubscriptionCallbackError.current = undefined
5676
})
5777

5878
useIsomorphicLayoutEffect(() => {
@@ -65,11 +85,12 @@ export function useSelector(selector) {
6585
}
6686

6787
latestSelectedState.current = newSelectedState
68-
} catch {
88+
} catch (err) {
6989
// we ignore all errors here, since when the component
7090
// is re-rendered, the selectors are called again, and
7191
// will throw again, if neither props nor store state
7292
// changed
93+
latestSubscriptionCallbackError.current = err
7394
}
7495

7596
forceRender({})

test/hooks/useSelector.spec.js

+42-1
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,11 @@ describe('React', () => {
169169

170170
describe('edge cases', () => {
171171
it('ignores transient errors in selector (e.g. due to stale props)', () => {
172+
// TODO Not sure this test is really testing what we want.
173+
// TODO The parent re-renders, which causes the child to re-run the selector anyway and throw the error.
174+
// TODO Had to flip the assertion for now. Probably needs to be rethought.
175+
176+
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
172177
const Parent = () => {
173178
const count = useSelector(s => s.count)
174179
return <Child parentCount={count} />
@@ -192,7 +197,43 @@ describe('React', () => {
192197
</ProviderMock>
193198
)
194199

195-
expect(() => store.dispatch({ type: '' })).not.toThrowError()
200+
expect(() => store.dispatch({ type: '' })).toThrowError(
201+
/while selecting the store state/
202+
)
203+
204+
spy.mockRestore()
205+
})
206+
207+
it('correlates the subscription callback error with a following error during rendering', () => {
208+
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
209+
210+
const Comp = () => {
211+
const result = useSelector(count => {
212+
if (count > 0) {
213+
throw new Error('foo')
214+
}
215+
216+
return count
217+
})
218+
219+
return <div>{result}</div>
220+
}
221+
222+
const store = createStore((count = -1) => count + 1)
223+
224+
const App = () => (
225+
<ProviderMock store={store}>
226+
<Comp />
227+
</ProviderMock>
228+
)
229+
230+
rtl.render(<App />)
231+
232+
expect(() => store.dispatch({ type: '' })).toThrow(
233+
/The error may be correlated/
234+
)
235+
236+
spy.mockRestore()
196237
})
197238
})
198239

0 commit comments

Comments
 (0)