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

Issue Reporters #3707

Merged
merged 4 commits into from
Feb 13, 2023
Merged

Issue Reporters #3707

merged 4 commits into from
Feb 13, 2023

Conversation

wbinnssmith
Copy link
Member

Big thanks to @jridgewell for helping me out with a number of Rust-isms with this change.

This expands handle_issues and the NextDevServerBuilder to accept an arbitrary IssueReporter -- a trait implementing report_issues which receives captured issues to send somewhere.

This replaces using a fixed ConsoleUi to send issues to stdout/stderr, though ConsoleUi now implements IssueReporter and is the default implementation of an issue reporter if no other is provided. It also moves the responsibility of detecting fatal errors out of ConsoleUi and into handle_issues itself.

This lays the foundation for alternative reporters, such as a test reporter to snapshot or assert against issues emitted, or a newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell justin@ridgewell.name

Big thanks to @jridgewell for helping me out with a number of Rust-isms with this change.

This expands `handle_issues` and the `NextDevServerBuilder` to accept an arbitrary `IssueReporter` -- a trait implementing `report_issues` which receives captured issues to send somewhere.

This replaces using a fixed `ConsoleUi` to send issues to stdout/stderr, though `ConsoleUi` now implements `IssueReporter` and is the default implementation of an issue reporter if no other is provided. It also moves the responsibility of detecting fatal errors out of `ConsoleUi` and into `handle_issues` itself.

This lays the foundation for alternative reporters, such as a test reporter to snapshot or assert against issues emitted, or a newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
@vercel
Copy link

vercel bot commented Feb 8, 2023

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

Name Status Preview Comments Updated
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-cra-web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 11:54PM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 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 Feb 9, 2023

Benchmark for 77a4fbe

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9332.72µs ± 59.38µs 9414.75µs ± 79.50µs +0.88%
bench_hmr_to_commit/Turbopack RCC/1000 modules 9545.09µs ± 90.78µs 9680.75µs ± 76.18µs +1.42%
bench_hmr_to_commit/Turbopack RSC/1000 modules 488.38ms ± 2.16ms 489.98ms ± 1.01ms +0.33%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9537.87µs ± 94.36µs 9545.10µs ± 88.55µs +0.08%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8418.57µs ± 46.50µs 8445.89µs ± 75.88µs +0.32%
bench_hmr_to_eval/Turbopack RCC/1000 modules 8526.81µs ± 82.19µs 8727.43µs ± 137.70µs +2.35%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8521.04µs ± 78.66µs 8596.51µs ± 73.84µs +0.89%
bench_hydration/Turbopack RCC/1000 modules 4166.92ms ± 7.63ms 4170.36ms ± 12.47ms +0.08%
bench_hydration/Turbopack RSC/1000 modules 3784.56ms ± 22.07ms 3772.99ms ± 19.48ms -0.31%
bench_hydration/Turbopack SSR/1000 modules 3710.75ms ± 9.35ms 3668.88ms ± 24.50ms -1.13%
bench_startup/Turbopack CSR/1000 modules 2760.95ms ± 6.20ms 2769.66ms ± 10.64ms +0.32%
bench_startup/Turbopack RCC/1000 modules 2542.53ms ± 11.32ms 2534.22ms ± 6.31ms -0.33%
bench_startup/Turbopack RSC/1000 modules 2442.54ms ± 8.49ms 2427.76ms ± 6.73ms -0.61%
bench_startup/Turbopack SSR/1000 modules 2108.57ms ± 2.51ms 2099.94ms ± 1.97ms -0.41%

crates/next-dev/src/lib.rs Outdated Show resolved Hide resolved
crates/next-dev/src/lib.rs Outdated Show resolved Hide resolved
crates/turbopack-dev-server/src/lib.rs Outdated Show resolved Hide resolved
crates/next-dev/src/lib.rs Outdated Show resolved Hide resolved
crates/next-dev/src/lib.rs Outdated Show resolved Hide resolved
crates/next-dev/src/lib.rs Outdated Show resolved Hide resolved
crates/turbopack-core/src/issue/mod.rs Outdated Show resolved Hide resolved
crates/turbopack-dev-server/src/lib.rs Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Benchmark for dea7d01

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.31ms ± 0.07ms 10.56ms ± 0.10ms +2.35%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.12ms ± 0.13ms 12.24ms ± 0.15ms +0.96%
bench_hmr_to_commit/Turbopack RSC/1000 modules 531.25ms ± 2.32ms 536.63ms ± 3.33ms +1.01%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.38ms ± 0.11ms 10.45ms ± 0.09ms +0.68%
bench_hmr_to_eval/Turbopack CSR/1000 modules 9210.96µs ± 73.79µs 9338.06µs ± 80.20µs +1.38%
bench_hmr_to_eval/Turbopack RCC/1000 modules 10.92ms ± 0.14ms 11.18ms ± 0.12ms +2.34%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9340.17µs ± 65.12µs 9444.67µs ± 68.29µs +1.12%
bench_hydration/Turbopack RCC/1000 modules 4405.98ms ± 9.24ms 4423.08ms ± 13.95ms +0.39%
bench_hydration/Turbopack RSC/1000 modules 4010.76ms ± 24.03ms 4014.78ms ± 16.61ms +0.10%
bench_hydration/Turbopack SSR/1000 modules 3851.96ms ± 35.69ms 3868.83ms ± 20.78ms +0.44%
bench_startup/Turbopack CSR/1000 modules 2891.22ms ± 9.56ms 2880.84ms ± 7.22ms -0.36%
bench_startup/Turbopack RCC/1000 modules 2671.45ms ± 8.73ms 2671.58ms ± 8.95ms +0.01%
bench_startup/Turbopack RSC/1000 modules 2572.71ms ± 10.54ms 2563.02ms ± 6.61ms -0.38%
bench_startup/Turbopack SSR/1000 modules 2203.74ms ± 5.20ms 2203.49ms ± 5.05ms -0.01%

@github-actions
Copy link
Contributor

Benchmark for 5f13588

Test Base PR % Significant %
bench_hydration/Turbopack RSC/1000 modules 4043.36ms ± 9.36ms 3981.56ms ± 15.85ms -1.53% -0.28%
bench_startup/Turbopack RSC/1000 modules 2549.25ms ± 13.61ms 2601.24ms ± 7.82ms +2.04% +0.35%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.04ms ± 0.09ms 10.17ms ± 0.08ms +1.26%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.24ms ± 0.34ms 11.97ms ± 0.17ms -2.21%
bench_hmr_to_commit/Turbopack RSC/1000 modules 519.85ms ± 1.79ms 523.68ms ± 2.34ms +0.74%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.08ms ± 0.12ms 10.16ms ± 0.07ms +0.74%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8989.60µs ± 92.95µs 9150.74µs ± 98.09µs +1.79%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.01ms ± 0.08ms 11.25ms ± 0.17ms +2.12%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9170.74µs ± 105.00µs 9201.67µs ± 98.14µs +0.34%
bench_hydration/Turbopack RCC/1000 modules 4424.88ms ± 10.97ms 4407.87ms ± 21.83ms -0.38%
bench_hydration/Turbopack RSC/1000 modules 4043.36ms ± 9.36ms 3981.56ms ± 15.85ms -1.53% -0.28%
bench_hydration/Turbopack SSR/1000 modules 3876.75ms ± 20.10ms 3867.03ms ± 22.48ms -0.25%
bench_startup/Turbopack CSR/1000 modules 2927.33ms ± 11.78ms 2898.23ms ± 17.45ms -0.99%
bench_startup/Turbopack RCC/1000 modules 2694.26ms ± 11.46ms 2669.23ms ± 9.51ms -0.93%
bench_startup/Turbopack RSC/1000 modules 2549.25ms ± 13.61ms 2601.24ms ± 7.82ms +2.04% +0.35%
bench_startup/Turbopack SSR/1000 modules 2203.42ms ± 3.44ms 2211.08ms ± 4.29ms +0.35%

@wbinnssmith wbinnssmith merged commit dae3e21 into main Feb 13, 2023
@wbinnssmith wbinnssmith deleted the wbinnssmith/issue-reporters branch February 13, 2023 16:07
Brooooooklyn added a commit that referenced this pull request Feb 14, 2023
Brooooooklyn added a commit that referenced this pull request Feb 14, 2023
This reverts commit dae3e21.

This commit makes the `turbotrace` hanging, x-ref:
https://vercel.slack.com/archives/C02UJN0A1UL/p1676369060652879
let task = tt.spawn_root_task(move || {
let console_ui = ConsoleUiVc::new(TransientInstance::new(LogOptions {
Copy link
Contributor

@jridgewell jridgewell Feb 14, 2023

Choose a reason for hiding this comment

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

This is the bug. The TransientInstance needs to be extracted out and shared by all calls, that way the same ConsoleUiVc will be reused. I mistakenly thought that this was an issue_reporter closure like the bug we found in test issue reporter.

let source = TransientValue::new(output.into());
let issues = IssueVc::peek_issues_with_path(output)
.await?
.strongly_consistent()
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, it might be this.

wbinnssmith added a commit that referenced this pull request Feb 14, 2023
jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
Big thanks to @jridgewell for helping me out with a number of Rust-isms
with this change.

This expands `handle_issues` and the `NextDevServerBuilder` to accept an
arbitrary `IssueReporter` -- a trait implementing `report_issues` which
receives captured issues to send somewhere.

This replaces using a fixed `ConsoleUi` to send issues to stdout/stderr,
though `ConsoleUi` now implements `IssueReporter` and is the default
implementation of an issue reporter if no other is provided. It also
moves the responsibility of detecting fatal errors out of `ConsoleUi`
and into `handle_issues` itself.

This lays the foundation for alternative reporters, such as a test
reporter to snapshot or assert against issues emitted, or a
newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
This reverts commit a8612abf5edbde3013d6fcf8845976f422cb2ffc.

This commit makes the `turbotrace` hanging, x-ref:
https://vercel.slack.com/archives/C02UJN0A1UL/p1676369060652879
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
Big thanks to @jridgewell for helping me out with a number of Rust-isms
with this change.

This expands `handle_issues` and the `NextDevServerBuilder` to accept an
arbitrary `IssueReporter` -- a trait implementing `report_issues` which
receives captured issues to send somewhere.

This replaces using a fixed `ConsoleUi` to send issues to stdout/stderr,
though `ConsoleUi` now implements `IssueReporter` and is the default
implementation of an issue reporter if no other is provided. It also
moves the responsibility of detecting fatal errors out of `ConsoleUi`
and into `handle_issues` itself.

This lays the foundation for alternative reporters, such as a test
reporter to snapshot or assert against issues emitted, or a
newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
This reverts commit a8612abf5edbde3013d6fcf8845976f422cb2ffc.

This commit makes the `turbotrace` hanging, x-ref:
https://vercel.slack.com/archives/C02UJN0A1UL/p1676369060652879
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
Big thanks to @jridgewell for helping me out with a number of Rust-isms
with this change.

This expands `handle_issues` and the `NextDevServerBuilder` to accept an
arbitrary `IssueReporter` -- a trait implementing `report_issues` which
receives captured issues to send somewhere.

This replaces using a fixed `ConsoleUi` to send issues to stdout/stderr,
though `ConsoleUi` now implements `IssueReporter` and is the default
implementation of an issue reporter if no other is provided. It also
moves the responsibility of detecting fatal errors out of `ConsoleUi`
and into `handle_issues` itself.

This lays the foundation for alternative reporters, such as a test
reporter to snapshot or assert against issues emitted, or a
newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
Big thanks to @jridgewell for helping me out with a number of Rust-isms
with this change.

This expands `handle_issues` and the `NextDevServerBuilder` to accept an
arbitrary `IssueReporter` -- a trait implementing `report_issues` which
receives captured issues to send somewhere.

This replaces using a fixed `ConsoleUi` to send issues to stdout/stderr,
though `ConsoleUi` now implements `IssueReporter` and is the default
implementation of an issue reporter if no other is provided. It also
moves the responsibility of detecting fatal errors out of `ConsoleUi`
and into `handle_issues` itself.

This lays the foundation for alternative reporters, such as a test
reporter to snapshot or assert against issues emitted, or a
newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
Big thanks to @jridgewell for helping me out with a number of Rust-isms
with this change.

This expands `handle_issues` and the `NextDevServerBuilder` to accept an
arbitrary `IssueReporter` -- a trait implementing `report_issues` which
receives captured issues to send somewhere.

This replaces using a fixed `ConsoleUi` to send issues to stdout/stderr,
though `ConsoleUi` now implements `IssueReporter` and is the default
implementation of an issue reporter if no other is provided. It also
moves the responsibility of detecting fatal errors out of `ConsoleUi`
and into `handle_issues` itself.

This lays the foundation for alternative reporters, such as a test
reporter to snapshot or assert against issues emitted, or a
newline-delimited JSON reporter for other tools to consume.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
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