Skip to content

Commit

Permalink
Preserve stack trace of errors inside store subscription callback
Browse files Browse the repository at this point in the history
Ported changes from react-redux-hooks-poc

Note: the "transient errors" test seems flawed atm.
  • Loading branch information
markerikson committed Apr 22, 2019
1 parent 9c7fc93 commit bc3d092
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 3 deletions.
25 changes: 23 additions & 2 deletions src/hooks/useSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,33 @@ export function useSelector(selector) {
contextSub
])

const latestSubscriptionCallbackError = useRef()
const latestSelector = useRef(selector)
const selectedState = selector(store.getState())

let selectedState = undefined

try {
selectedState = latestSelector.current(store.getState())
} catch (err) {
let errorMessage = `An error occured while selecting the store state: ${
err.message
}.`

if (latestSubscriptionCallbackError.current) {
errorMessage += `\nThe error may be correlated with this previous error:\n${
latestSubscriptionCallbackError.current.stack
}\n\nOriginal stack trace:`
}

throw new Error(errorMessage)
}

const latestSelectedState = useRef(selectedState)

useIsomorphicLayoutEffect(() => {
latestSelector.current = selector
latestSelectedState.current = selectedState
latestSubscriptionCallbackError.current = undefined
})

useIsomorphicLayoutEffect(() => {
Expand All @@ -65,11 +85,12 @@ export function useSelector(selector) {
}

latestSelectedState.current = newSelectedState
} catch {
} catch (err) {
// we ignore all errors here, since when the component
// is re-rendered, the selectors are called again, and
// will throw again, if neither props nor store state
// changed
latestSubscriptionCallbackError.current = err
}

forceRender({})
Expand Down
43 changes: 42 additions & 1 deletion test/hooks/useSelector.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ describe('React', () => {

describe('edge cases', () => {
it('ignores transient errors in selector (e.g. due to stale props)', () => {
// TODO Not sure this test is really testing what we want.
// TODO The parent re-renders, which causes the child to re-run the selector anyway and throw the error.
// TODO Had to flip the assertion for now. Probably needs to be rethought.

const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
const Parent = () => {
const count = useSelector(s => s.count)
return <Child parentCount={count} />
Expand All @@ -192,7 +197,43 @@ describe('React', () => {
</ProviderMock>
)

expect(() => store.dispatch({ type: '' })).not.toThrowError()
expect(() => store.dispatch({ type: '' })).toThrowError(
/while selecting the store state/
)

spy.mockRestore()
})

it('correlates the subscription callback error with a following error during rendering', () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})

const Comp = () => {
const result = useSelector(count => {
if (count > 0) {
throw new Error('foo')
}

return count
})

return <div>{result}</div>
}

const store = createStore((count = -1) => count + 1)

const App = () => (
<ProviderMock store={store}>
<Comp />
</ProviderMock>
)

rtl.render(<App />)

expect(() => store.dispatch({ type: '' })).toThrow(
/The error may be correlated/
)

spy.mockRestore()
})
})

Expand Down

0 comments on commit bc3d092

Please sign in to comment.