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(core): better error logging on SSR/dev failures + log stacktraces and error causes #8872

Merged
merged 3 commits into from Apr 7, 2023

Conversation

slorber
Copy link
Collaborator

@slorber slorber commented Apr 7, 2023

Motivation

Currently when a React component fails to render, we have bad error messages related to chalk (see #8826 (comment)):

12:50:33 PM: [INFO] [en] Creating an optimized production build...
12:50:50 PM: [info] [webpackbar] Compiling Client
12:50:50 PM: [info] [webpackbar] Compiling Server
12:51:02 PM: [success] [webpackbar] Client: Compiled successfully in 11.76s
12:51:20 PM: [success] [webpackbar] Server: Compiled with some errors in 30.14s
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: TypeError: _chalk.default.bold is not a function
12:51:20 PM: [ERROR] Unable to build website for locale en.
12:51:20 PM: [ERROR] Error: Failed to compile with errors.
12:51:20 PM:     at /opt/build/repo/packages/docusaurus/lib/webpack/utils.js:180:24
12:51:20 PM:     at /opt/build/repo/node_modules/webpack/lib/MultiCompiler.js:554:14
12:51:20 PM:     at processQueueWorker (/opt/build/repo/node_modules/webpack/lib/MultiCompiler.js:491:6)
12:51:20 PM:     at processTicksAndRejections (node:internal/process/task_queues:78:11)

For some reason when using chalk inside the Webpack serverEntry.tsx we have issues (looks related to unability to resolve properly "ansi-styles" package). I was not able to fix this issue so simply removed the usage of chalk in serverEntry.tsx

I also did some cleanup in the way we output Webpack stats errors/warnings in start/build.

Test Plan

CI and local tests in both dev/prod mode

DOCUSAURUS_CRASH_TEST=true yarn build:website:fast

CleanShot 2023-04-07 at 18 03 03@2x


Some additional notes.

  • It seems that nested stacktraces (error causes) emitted during the eval SSR process are lost in the parent Node context. Not sure how to propagate error causes upper in the tree so that we can get the causes in the Webpack errors.
  • The Docusaurus logger is annoying when logging errors, it stringifies objects before adding colors and this does not take into consideration the error causes that Nodejs is normally able to log properly. I'm not sure how to add red color to the native display of error causes that Nodejs supports. Maybe it's better to have errors without colors but the full causal chain 🤷‍♂️

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 7, 2023
Comment on lines 247 to 251
// TODO this logger thing is annoying
// This makes it impossible to use native logging of nested stacktraces
// ie the error causes are not logged here
// It seems we can't have colors + nested stacktrace logging at the same time
logger.error(err instanceof Error ? err.stack : err);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Josh-Cena this thing is a bit annoying because if an error is thrown with a cause it won't print the cause

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we can use the built-in error logging? This code has been there forever, and I think it's fine at this point to remove it and just use console.log(err) instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do that then, doesn't look too bad:

CleanShot 2023-04-07 at 18 42 43@2x

@netlify
Copy link

netlify bot commented Apr 7, 2023

[V2]

Name Link
🔨 Latest commit 09acbb3
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/643048132d8a340008cb01e9
😎 Deploy Preview https://deploy-preview-8872--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟢 91 🟢 97 🟢 92 🟢 100 🟢 90 Report
/docs/installation 🟠 80 🟢 100 🟢 92 🟢 100 🟢 90 Report

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

Size Change: +306 B (0%)

Total Size: 1.01 MB

Filename Size Change
website/build/assets/js/main.********.js 752 kB +306 B (0%)
ℹ️ View Unchanged
Filename Size
website/.docusaurus/globalData.json 101 kB
website/build/assets/css/styles.********.css 113 kB
website/build/index.html 41.2 kB

compressed-size-action

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Apr 7, 2023
@slorber
Copy link
Collaborator Author

slorber commented Apr 7, 2023

Let's give it a try.

Not sure I'll backport this, looks a bit risky although I'd be happy to have better error logging in the v2.x releases.

We'll see if it works great first

@slorber slorber changed the title fix(core): better error logging on SSR/dev failures fix(core): better error logging on SSR/dev failures + log stacktraces and error causes Apr 7, 2023
@slorber slorber merged commit a9a5f89 into main Apr 7, 2023
29 checks passed
@slorber slorber deleted the slorber/fix-serverEntry-chalk branch April 7, 2023 17:01
This was referenced Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants