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(build): avoid logging an object on compilation errors #15885

Merged
merged 3 commits into from
Jun 30, 2022

Conversation

BlackHole1
Copy link
Contributor

When building with webpack, all information from stats will be printed when an error occurs.

图片

In my project, stats will print out over 5,000 lines, which is hard to swallow..

@nx-cloud
Copy link

nx-cloud bot commented Aug 20, 2021

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 8b4f8ee. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@stale
Copy link

stale bot commented Jan 9, 2022

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@ndelangen
Copy link
Member

Hello @BlackHole1 really appreciate the assist here, could you show us the error occurring somewhere perhaps?
Does it happen in one of the examples in this monorepo?

@BlackHole1
Copy link
Contributor Author

Hello @BlackHole1 really appreciate the assist here, could you show us the error occurring somewhere perhaps? Does it happen in one of the examples in this monorepo?

yes, the project uses monorepo.

@BlackHole1
Copy link
Contributor Author

@ndelangen
Copy link
Member

I think the true reason we're logging this stat is because of this: (we are throwing here with the stats

return fail(stats);

and then this: (we are awaiting these 2 promises (the preview is throwing)

const [managerStats, previewStats] = await Promise.all([manager, preview]);

and then finally the thrown stats object is caught here, and logged as an error:

I think we should throw with a generic error instead here:

return fail(stats);

          return fail(new Error('compiler encountered errors builder the preview'));

@BlackHole1 could you adjust the PR?
mind we have a similar code path in the builder-webpack5:

compiler.close(() =>
options.debugWebpack
? fail(stats)
: fail(new Error('=> Webpack failed, learn more with --debug-webpack'))

and also for the manager builders:

fail(error || stats);

fail(error || stats);

@BlackHole1
Copy link
Contributor Author

@ndelangen Great! I will be making adjustments later today.

@ndelangen
Copy link
Member

@yannbf this seems relevant for our changes we were making today

# Conflicts:
#	lib/core-server/src/build-static.ts
@ndelangen ndelangen merged commit 3c7d933 into storybookjs:next Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants