[BUG] Reuse Server Correlation ID when Hydrating Error Pages. #846
Conversation
| if (isInitialPageRef.current && window.__ERROR__) { | ||
| isInitialPageRef.current = false | ||
| return window.__INITIAL_CORRELATION_ID__ | ||
| } |
There was a problem hiding this comment.
For my own eduction here, this (the useRef) is another one of our tricks to have various React lifecycle things be aware of first (SSR) vs. second (CSR) render passes?
There was a problem hiding this comment.
I wouldn't say this is something in the bag of tricks. The reason I used useRef here was because the alternative is using a global variable. The solution here is pretty much specific to the problem at hand, which is that we don't want to generated a new CSR correlation id when the thing we are rendering is an error page that happened on the server.
|
@bendvc falling tests on this one 😔 tried to merge develop in for the circle changes but no dice |
|
@bfeister It looks like it has linting issues https://app.circleci.com/pipelines/github/SalesforceCommerceCloud/pwa-kit/5493/workflows/afe061a6-60b4-4c59-aaa7-cc647c95e9ca/jobs/37088 |
| @@ -71,7 +71,7 @@ const OuterApp = ({routes, error, WrappedApp, locals}) => { | |||
| OuterApp.propTypes = { | |||
| routes: PropTypes.array.isRequired, | |||
| error: PropTypes.object, | |||
| WrappedApp: PropTypes.element.isRequired, | |||
| WrappedApp: PropTypes.func.isRequired, | |||
There was a problem hiding this comment.
So are we moving from WrappedApp being an element to being a SFC (stateless functional component)?
There was a problem hiding this comment.
It is a HOC actually, the wrappedApp is supposed to be a RouteComponent what is wrapped with some HOC.
| describe('main', function() { | ||
| test('OuterApp renders without error', () => { | ||
| uuidv4.mockReturnValueOnce('7f21aea5-6962-4162-8204-9da85c802022') | ||
| const oldPreloadedState = window.__PRELOADED_STATE__ | ||
| window.__PRELOADED_STATE__ = { | ||
| appProps: {} | ||
| } | ||
| const App = () => <div>App</div> | ||
| const locals = {} | ||
| const props = { | ||
| error: undefined, | ||
| locals, | ||
| routes: getRoutes(locals), | ||
| WrappedApp: routeComponent(App, false, locals) | ||
| } | ||
| const wrapper = mount(<OuterApp {...props} />) | ||
| expect(wrapper.find(App).length).toBe(1) | ||
| window.__PRELOADED_STATE__ = oldPreloadedState | ||
| }) | ||
|
|
||
| test('OuterApp triggers the error page when there is an error', () => { | ||
| const oldWindowError = window.__ERROR__ | ||
| window.__ERROR__ = new errors.HTTPNotFound('Not found') | ||
| const App = () => <div>App</div> | ||
| const locals = {} | ||
| const props = { | ||
| error: window.__ERROR__, | ||
| locals, | ||
| routes: getRoutes(locals), | ||
| WrappedApp: routeComponent(App, false, locals) | ||
| } | ||
| const wrapper = mount(<OuterApp {...props} />) | ||
| expect(wrapper.find(AppErrorBoundary).length).toBe(1) | ||
| window.__ERROR__ = oldWindowError | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Great work! 🏅 Small gradual improvements to test coverage are the way we want to do better over time
Description
Currently if an error happens on the server and an error page is rendered, you will get a React text mismatch error. This is the result of a new correlation id being generated on the client that doesn't match the error on the server.
To remedy this we need to be displaying the correlation id that is related to the request that caused an error to be rendered and not generating a new one.
Solution
To achieve the above desired outcome I've made the following changes:
__INITIAL_CORRELATION_ID__Steps to Reproduce
const product = useProduct({id: '25502228Mxxx'}, {useErrorBoundary: true})http://localhost:3000/query-errorsTypes of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization