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

Error handling fixes for turbopack-node and the HMR runtime #5189

Merged
merged 10 commits into from
Jun 8, 2023

Conversation

ForsakenHarmony
Copy link
Contributor

@ForsakenHarmony ForsakenHarmony commented Jun 2, 2023

Description

  • errors during module init are no longer ignored
  • non-Errors thrown inside node.js now display correctly

Bonus:

  • you can now debug the spawned node.js processes without recompiling via TURBOPACK_DEBUG_JS=operation

Fixes WEB-997

@vercel
Copy link

vercel bot commented Jun 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-tailwind-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jun 7, 2023 7:08pm
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jun 7, 2023 7:08pm
9 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-cra-web ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 7:08pm

@ForsakenHarmony ForsakenHarmony changed the title Hrmny/web 1143 bubble up module init errors Error handling fixes for turbopack-node and the HMR runtime Jun 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

✅ This change can build next-swc

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

Linux Benchmark for 66ee3ea

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9252.14µs ± 48.80µs 9234.41µs ± 51.46µs -0.19%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8494.73µs ± 79.02µs 8471.73µs ± 145.05µs -0.27%
bench_startup/Turbopack CSR/1000 modules 909.20ms ± 2.39ms 902.90ms ± 6.38ms -0.69%

@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-1143-bubble-up-module-init-errors branch from 7660790 to cdadd15 Compare June 5, 2023 19:07
@ForsakenHarmony ForsakenHarmony marked this pull request as ready for review June 5, 2023 19:27
@ForsakenHarmony ForsakenHarmony requested a review from a team as a code owner June 5, 2023 19:27
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2023

Linux Benchmark for 61e7da1

Test Base PR % Significant %
bench_startup/Turbopack CSR/1000 modules 896.58ms ± 2.24ms 884.58ms ± 3.04ms -1.34% -0.16%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8341.89µs ± 36.66µs 8367.80µs ± 34.26µs +0.31%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7511.89µs ± 68.28µs 8136.00µs ± 674.08µs +8.31%
bench_startup/Turbopack CSR/1000 modules 896.58ms ± 2.24ms 884.58ms ± 3.04ms -1.34% -0.16%

crates/turbopack-node/src/debug.rs Outdated Show resolved Hide resolved
@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-1143-bubble-up-module-init-errors branch from 1d73fd2 to dfd440d Compare June 5, 2023 21:31
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2023

Linux Benchmark for 8806d6c

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9594.93µs ± 67.81µs 9628.42µs ± 86.51µs +0.35%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8833.14µs ± 83.21µs 8744.03µs ± 61.15µs -1.01%
bench_startup/Turbopack CSR/1000 modules 934.01ms ± 2.81ms 940.74ms ± 4.94ms +0.72%

crates/turbopack-node/js/src/ipc/error.ts Outdated Show resolved Hide resolved
@@ -133,9 +136,11 @@ function createIpc<TIncoming, TOutgoing>(
...structuredError(error),
});
} catch (err) {
console.error("failed to send error back to rust:", err);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should only be logged in some kind of verbose/tracing mode, as I don't think it's very actionable from a user's perspective.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this because previously the error outputted was something like "unexpected EOF" so while it might not be actionable I thought it might be easier to understand and if we receive feedback easier to debug, can remove it again though

crates/turbopack-node/src/debug.rs Show resolved Hide resolved
@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-1143-bubble-up-module-init-errors branch 2 times, most recently from 823e706 to e637d54 Compare June 7, 2023 18:32
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Linux Benchmark for 6a402af

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9589.03µs ± 67.70µs 9703.33µs ± 82.75µs +1.19%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8608.67µs ± 53.46µs 9078.08µs ± 318.93µs +5.45%
bench_startup/Turbopack CSR/1000 modules 932.44ms ± 0.83ms 927.71ms ± 2.74ms -0.51%

@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-1143-bubble-up-module-init-errors branch from e637d54 to 30aa8da Compare June 7, 2023 19:08
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Linux Benchmark for de987a9

Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9195.14µs ± 88.16µs 8795.36µs ± 61.32µs -4.35% -1.12%
bench_startup/Turbopack CSR/1000 modules 915.55ms ± 1.04ms 905.71ms ± 3.83ms -1.08% -0.01%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9195.14µs ± 88.16µs 8795.36µs ± 61.32µs -4.35% -1.12%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8044.01µs ± 31.93µs 8094.71µs ± 172.32µs +0.63%
bench_startup/Turbopack CSR/1000 modules 915.55ms ± 1.04ms 905.71ms ± 3.83ms -1.08% -0.01%

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Windows Benchmark for de987a9

Test Base PR % Significant %
bench_startup/Turbopack CSR/1000 modules 2977.93ms ± 12.07ms 3038.23ms ± 17.64ms +2.02% +0.03%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 15.51ms ± 0.01ms 15.50ms ± 0.01ms -0.03%
bench_hmr_to_eval/Turbopack CSR/1000 modules 15.51ms ± 0.01ms 15.50ms ± 0.01ms -0.08%
bench_startup/Turbopack CSR/1000 modules 2977.93ms ± 12.07ms 3038.23ms ± 17.64ms +2.02% +0.03%

@ForsakenHarmony ForsakenHarmony merged commit 8f8a14c into main Jun 8, 2023
@ForsakenHarmony ForsakenHarmony deleted the hrmny/web-1143-bubble-up-module-init-errors branch June 8, 2023 00:03
ForsakenHarmony added a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…/turborepo#5189)

### Description
- errors during module init are no longer ignored
- non-Errors thrown inside node.js now display correctly

Bonus:
- you can now debug the spawned node.js processes without recompiling
via `TURBOPACK_DEBUG_JS=operation`
ForsakenHarmony added a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…/turborepo#5189)

### Description
- errors during module init are no longer ignored
- non-Errors thrown inside node.js now display correctly

Bonus:
- you can now debug the spawned node.js processes without recompiling
via `TURBOPACK_DEBUG_JS=operation`
ForsakenHarmony added a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…/turborepo#5189)

### Description
- errors during module init are no longer ignored
- non-Errors thrown inside node.js now display correctly

Bonus:
- you can now debug the spawned node.js processes without recompiling
via `TURBOPACK_DEBUG_JS=operation`
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.

3 participants