Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: context.error was not printed because JSON.stringify(Error) doesn't work by default in JS #19217

Conversation

Platonn
Copy link
Contributor

@Platonn Platonn commented Sep 9, 2024

Previously, the logged message Request is resolved with the SSR rendering error was printed by OptimizedSsrEngine alongside with the JSON.strinfied property context.error which ended up looking like an empty object {} (even if it was not empty, but a real Error object instead!). It was because of a natural thing in JavaScript: JSON.stringify() invoked on object of type Error always returns {} (it's becasue properties of the Error object are non-enumerable; for more see https://stackoverflow.com/a/68640559/11734692).

Now the context.error property is printed as a pretty error string. How it's done: the context.error property is first converted into a pretty string with the help of the formatWithOptions() function of native node:util package. Btw. formatWithOptions() is the same mechanism that is used by the console.log() under the hood to convert a given object into a string.
And when performing later the JSON.stringify() on the context object, the context.error property has been already converted into a pretty string describing an error. So JSON.stringify simply re-returns this string. See the following example of a pretty error string:

CmsPageNotFoundOutboundHttpError: CMS Page Not Found
    at HttpErrorHandlerInterceptor2.handleError (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:1695890)
    at Object.error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:1695743)
    at /opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:585551
    at OperatorSubscriber2._this._error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:507051)
    at Subscriber2.error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:460681)
    at OperatorSubscriber2._this._error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:507088)
    at Subscriber2.error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:460681)
    at Subscriber2._error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:461147)
    at Subscriber2.error (/opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:460681)
    at /opt/app/spartacusstore/dist/spartacusstore/server/main.js:1:585584 {
  [cause]: HttpErrorResponse { headers: HttpHeaders { normalizedNames: [Map], lazyUpdate: null, headers: [Map] }, status: 404, statusText: 'Unknown Error', url: 'https://api.cg79x9wuu9-eccommerc1-p5-public.model-t.myhybris.cloud/occ/v2/electronics-spa/cms/pages2?pageType=ContentPage&pageLabelOrId=%2Fnot-found&lang=en&curr=USD', ok: false, name: 'HttpErrorResponse', message: 'Http failure response for https://api.cg79x9wuu9-eccommerc1-p5-public.model-t.myhybris.cloud/occ/v2/electronics-spa/cms/pages2?pageType=ContentPage&pageLabelOrId=%2Fnot-found&lang=en&curr=USD: 404 ', error: { errors: [Array] } }
}

As you can see in the example above, some nested properties of the [cause] property, are described by Array or Map. This has been fixed also in this PR, by configuring depth: 10 (instead of the default 2) in the options passed to the formatWithOptions() util.

TODO:

  • add unit tests

closes https://jira.tools.sap/browse/CXSPA-8360

Base automatically changed from epic/ssr-error-handling to develop September 10, 2024 18:41
@Platonn Platonn changed the base branch from develop to epic/ssr-error-handling September 18, 2024 10:54
@Platonn
Copy link
Contributor Author

Platonn commented Sep 18, 2024

closing in favor of #19259

@Platonn Platonn closed this Sep 18, 2024
@Platonn Platonn deleted the epic/ssr-error-handling--print-context-error branch September 18, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant