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

Replace unreachable with bail in finalize_css #8694

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

bgw
Copy link
Member

@bgw bgw commented Jul 8, 2024

Description

@rauchg reported:

thread 'tokio-runtime-worker' panicked at /Users/geist/.cargo/git/checkouts/turbo-df7a549334890fa5/2af6b12/crates/turbopack-css/src/process.rs:439:22:
internal error: entered unreachable code: this case should be filtered out while parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

(https://vercel.slack.com/archives/C01A2M9R8RZ/p1720449958135569)

A review of the code confirms that this should be unreachable under normal circumstances:

There's a check here that FileContent::Content exists before constructing the ParseCssResult::Ok that's needed to reach the line that contains the unreachable! assertion: https://github.com/vercel/turbo/blob/main/crates/turbopack-css/src/process.rs#L491

But there's no guarantee that the view of the world is strongly consistent across multiple Vcs inside most tasks. Some Vcs may no longer exist due to invalidation (or cache eviction once that's enabled) while this task was running.

There is a guarantee that at the top-level of the application that there's strong (eventual) consistency, so we can be confident that this bail! won't bubble all the way up.

Testing Instructions

cargo nextest r -p 'turbopack-*'

Copy link

vercel bot commented Jul 8, 2024

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

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 8, 2024 6:15pm
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 8, 2024 6:15pm
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:15pm

Copy link
Member Author

bgw commented Jul 8, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bgw and the rest of your teammates on Graphite Graphite

Copy link
Contributor

github-actions bot commented Jul 8, 2024

⚠️ This change may fail to build next-swc.

Logs

error: failed to select a version for `either`.
    ... required by package `browserslist-rs v0.16.0`
    ... which satisfies dependency `browserslist-rs = "^0.16.0"` of package `turbopack-core v0.1.0 (https://github.com/vercel/turbo?rev=3cfd0b81bf57b318194913bd7b84a9ba12a2106c#57b5349a)`
    ... which satisfies git dependency `turbopack-core` of package `turbopack-binding v0.1.0 (https://github.com/vercel/turbo?rev=3cfd0b81bf57b318194913bd7b84a9ba12a2106c#57b5349a)`
    ... which satisfies git dependency `turbopack-binding` (locked to 0.1.0) of package `next-build-test v0.1.0 (/root/actions-runner/_work/turbo/turbo/packages/next-swc/crates/next-build-test)`
versions that meet the requirements `^1.12` are: 1.13.0, 1.12.0

all possible versions conflict with previously selected packages.

  previously selected package `either v1.10.0`
    ... which satisfies dependency `either = "^1"` (locked to 1.10.0) of package `next-custom-transforms v0.0.0 (/root/actions-runner/_work/turbo/turbo/packages/next-swc/crates/next-custom-transforms)`
    ... which satisfies path dependency `next-custom-transforms` (locked to 0.0.0) of package `next-swc-napi v0.0.0 (/root/actions-runner/_work/turbo/turbo/packages/next-swc/crates/napi)`

failed to select a version for `either` which could resolve this conflict

See job summary for details

Copy link
Contributor

github-actions bot commented Jul 8, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented Jul 8, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

@bgw bgw marked this pull request as ready for review July 8, 2024 18:26
@bgw bgw requested a review from a team as a code owner July 8, 2024 18:26
@bgw bgw requested a review from sokra July 8, 2024 18:34
@bgw bgw assigned sokra Jul 8, 2024
@bgw bgw merged commit 0ac858f into main Jul 8, 2024
55 of 58 checks passed
@bgw bgw deleted the bgw/finalize-css-unreachable branch July 8, 2024 19:57
kdy1 added a commit to vercel/next.js that referenced this pull request Jul 12, 2024
# Turbopack
* vercel/turborepo#8686 <!-- Tobias Koppers -
improve performance of the graph aggregation -->
* vercel/turborepo#8694 <!-- Benjamin Woodruff -
Replace `unreachable` with `bail` in `finalize_css` -->
* vercel/turborepo#8661 <!-- Benjamin Woodruff -
Create a minimum viable `ResolvedVc` type -->
* vercel/turborepo#8662 <!-- Benjamin Woodruff - Add
a minimum viable ResolvedValue marker trait -->
* vercel/turborepo#8678 <!-- Benjamin Woodruff - Add
derive macro for ResolvedValue -->
* vercel/turborepo#8715 <!-- Benjamin Woodruff -
Switch RcStr from std::sync::Arc to triomphe::Arc -->
* vercel/turborepo#8699 <!-- Donny/강동윤 - perf: Merge
multiple `EsmBinding` -->
* vercel/turborepo#8720 <!-- Benjamin Woodruff - Add
support for `#[turbo_tasks::value(resolved)]` and
`#[turbo_tasks::value_trait(resolved)]` -->
* vercel/turborepo#8706 <!-- Donny/강동윤 - build:
Update `swc_core` to `v0.96.9` -->

### What?

Update SWC crates to
swc-project/swc@226617e
and
swc-project/plugins@b7658c3

### Why?

To keep in sync

### How?

 - Closes #64890
 - Closes #63104
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…#8694)

### Description

@rauchg reported:

```
thread 'tokio-runtime-worker' panicked at /Users/geist/.cargo/git/checkouts/turbo-df7a549334890fa5/f5fc980/crates/turbopack-css/src/process.rs:439:22:
internal error: entered unreachable code: this case should be filtered out while parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

(https://vercel.slack.com/archives/C01A2M9R8RZ/p1720449958135569)

A review of the code confirms that this should be unreachable under normal circumstances:

> There's a check here that `FileContent::Content` exists before constructing the `ParseCssResult::Ok` that's needed to reach the line that contains the `unreachable!` assertion: https://github.com/vercel/turbo/blob/main/crates/turbopack-css/src/process.rs#L491

But there's no guarantee that the view of the world is strongly consistent across multiple `Vc`s inside most tasks. Some `Vc`s may no longer exist due to invalidation (or cache eviction once that's enabled) while this task was running.

There is a guarantee that at the top-level of the application that there's strong (eventual) consistency, so we can be confident that this `bail!` won't bubble all the way up.
 
### Testing Instructions

`cargo nextest r -p 'turbopack-*'`
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…#8694)

### Description

@rauchg reported:

```
thread 'tokio-runtime-worker' panicked at /Users/geist/.cargo/git/checkouts/turbo-df7a549334890fa5/075742d/crates/turbopack-css/src/process.rs:439:22:
internal error: entered unreachable code: this case should be filtered out while parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

(https://vercel.slack.com/archives/C01A2M9R8RZ/p1720449958135569)

A review of the code confirms that this should be unreachable under normal circumstances:

> There's a check here that `FileContent::Content` exists before constructing the `ParseCssResult::Ok` that's needed to reach the line that contains the `unreachable!` assertion: https://github.com/vercel/turbo/blob/main/crates/turbopack-css/src/process.rs#L491

But there's no guarantee that the view of the world is strongly consistent across multiple `Vc`s inside most tasks. Some `Vc`s may no longer exist due to invalidation (or cache eviction once that's enabled) while this task was running.

There is a guarantee that at the top-level of the application that there's strong (eventual) consistency, so we can be confident that this `bail!` won't bubble all the way up.
 
### Testing Instructions

`cargo nextest r -p 'turbopack-*'`
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…#8694)

### Description

@rauchg reported:

```
thread 'tokio-runtime-worker' panicked at /Users/geist/.cargo/git/checkouts/turbo-df7a549334890fa5/bd9ac32/crates/turbopack-css/src/process.rs:439:22:
internal error: entered unreachable code: this case should be filtered out while parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

(https://vercel.slack.com/archives/C01A2M9R8RZ/p1720449958135569)

A review of the code confirms that this should be unreachable under normal circumstances:

> There's a check here that `FileContent::Content` exists before constructing the `ParseCssResult::Ok` that's needed to reach the line that contains the `unreachable!` assertion: https://github.com/vercel/turbo/blob/main/crates/turbopack-css/src/process.rs#L491

But there's no guarantee that the view of the world is strongly consistent across multiple `Vc`s inside most tasks. Some `Vc`s may no longer exist due to invalidation (or cache eviction once that's enabled) while this task was running.

There is a guarantee that at the top-level of the application that there's strong (eventual) consistency, so we can be confident that this `bail!` won't bubble all the way up.
 
### Testing Instructions

`cargo nextest r -p 'turbopack-*'`
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…#8694)

### Description

@rauchg reported:

```
thread 'tokio-runtime-worker' panicked at /Users/geist/.cargo/git/checkouts/turbo-df7a549334890fa5/3c7fb4e/crates/turbopack-css/src/process.rs:439:22:
internal error: entered unreachable code: this case should be filtered out while parsing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

(https://vercel.slack.com/archives/C01A2M9R8RZ/p1720449958135569)

A review of the code confirms that this should be unreachable under normal circumstances:

> There's a check here that `FileContent::Content` exists before constructing the `ParseCssResult::Ok` that's needed to reach the line that contains the `unreachable!` assertion: https://github.com/vercel/turbo/blob/main/crates/turbopack-css/src/process.rs#L491

But there's no guarantee that the view of the world is strongly consistent across multiple `Vc`s inside most tasks. Some `Vc`s may no longer exist due to invalidation (or cache eviction once that's enabled) while this task was running.

There is a guarantee that at the top-level of the application that there's strong (eventual) consistency, so we can be confident that this `bail!` won't bubble all the way up.
 
### Testing Instructions

`cargo nextest r -p 'turbopack-*'`
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 14, 2024
* vercel/turborepo#8686 <!-- Tobias Koppers -
improve performance of the graph aggregation -->
* vercel/turborepo#8694 <!-- Benjamin Woodruff -
Replace `unreachable` with `bail` in `finalize_css` -->
* vercel/turborepo#8661 <!-- Benjamin Woodruff -
Create a minimum viable `ResolvedVc` type -->
* vercel/turborepo#8662 <!-- Benjamin Woodruff - Add
a minimum viable ResolvedValue marker trait -->
* vercel/turborepo#8678 <!-- Benjamin Woodruff - Add
derive macro for ResolvedValue -->
* vercel/turborepo#8715 <!-- Benjamin Woodruff -
Switch RcStr from std::sync::Arc to triomphe::Arc -->
* vercel/turborepo#8699 <!-- Donny/강동윤 - perf: Merge
multiple `EsmBinding` -->
* vercel/turborepo#8720 <!-- Benjamin Woodruff - Add
support for `#[turbo_tasks::value(resolved)]` and
`#[turbo_tasks::value_trait(resolved)]` -->
* vercel/turborepo#8706 <!-- Donny/강동윤 - build:
Update `swc_core` to `v0.96.9` -->

Update SWC crates to
swc-project/swc@226617e
and
swc-project/plugins@b7658c3

To keep in sync

 - Closes #64890
 - Closes #63104
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 15, 2024
* vercel/turborepo#8686 <!-- Tobias Koppers -
improve performance of the graph aggregation -->
* vercel/turborepo#8694 <!-- Benjamin Woodruff -
Replace `unreachable` with `bail` in `finalize_css` -->
* vercel/turborepo#8661 <!-- Benjamin Woodruff -
Create a minimum viable `ResolvedVc` type -->
* vercel/turborepo#8662 <!-- Benjamin Woodruff - Add
a minimum viable ResolvedValue marker trait -->
* vercel/turborepo#8678 <!-- Benjamin Woodruff - Add
derive macro for ResolvedValue -->
* vercel/turborepo#8715 <!-- Benjamin Woodruff -
Switch RcStr from std::sync::Arc to triomphe::Arc -->
* vercel/turborepo#8699 <!-- Donny/강동윤 - perf: Merge
multiple `EsmBinding` -->
* vercel/turborepo#8720 <!-- Benjamin Woodruff - Add
support for `#[turbo_tasks::value(resolved)]` and
`#[turbo_tasks::value_trait(resolved)]` -->
* vercel/turborepo#8706 <!-- Donny/강동윤 - build:
Update `swc_core` to `v0.96.9` -->

Update SWC crates to
swc-project/swc@226617e
and
swc-project/plugins@b7658c3

To keep in sync

 - Closes #64890
 - Closes #63104
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 16, 2024
* vercel/turborepo#8686 <!-- Tobias Koppers -
improve performance of the graph aggregation -->
* vercel/turborepo#8694 <!-- Benjamin Woodruff -
Replace `unreachable` with `bail` in `finalize_css` -->
* vercel/turborepo#8661 <!-- Benjamin Woodruff -
Create a minimum viable `ResolvedVc` type -->
* vercel/turborepo#8662 <!-- Benjamin Woodruff - Add
a minimum viable ResolvedValue marker trait -->
* vercel/turborepo#8678 <!-- Benjamin Woodruff - Add
derive macro for ResolvedValue -->
* vercel/turborepo#8715 <!-- Benjamin Woodruff -
Switch RcStr from std::sync::Arc to triomphe::Arc -->
* vercel/turborepo#8699 <!-- Donny/강동윤 - perf: Merge
multiple `EsmBinding` -->
* vercel/turborepo#8720 <!-- Benjamin Woodruff - Add
support for `#[turbo_tasks::value(resolved)]` and
`#[turbo_tasks::value_trait(resolved)]` -->
* vercel/turborepo#8706 <!-- Donny/강동윤 - build:
Update `swc_core` to `v0.96.9` -->

Update SWC crates to
swc-project/swc@226617e
and
swc-project/plugins@b7658c3

To keep in sync

 - Closes #64890
 - Closes #63104
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.

2 participants