Empty TURBO_API causes crash with turbo >=1.11.0
#6921
Assignees
Labels
kind: bug
Something isn't working
needs: triage
New issues get this label. Remove it after triage
owned-by: turborepo
Comments
ihmpavel
added
kind: bug
|
I would guess that this stems from Alpine using musl instead of glibc, but it's odd that you have been able to reproduce on your Macbook. |
|
Are you setting |
|
Hi, thank you for you insights! I tried building multiple times again, with latest canary release locally on my MacBook outside of Docker, and it looks like it is working again. In the After deleting the last two lines, the build started working again. The build command is only I tried reading changelog, but I do not see any change regarding I am currently testing this on CI/CD, whether not specifying empty TURBO_API variable/argument in Dockerfile solves the error there. The ideal way will be the same behavior as in previous minor version. Can you pinpoint me to the source code, where I can find the change? If it will be something trivial, I can try creating PR reverting/fixing this behavior, but I have never worked with Rust before. |
Merged
Thank you so much for this! It looks like we accidentally changed the interpretation of |
ihmpavel
changed the title
TURBO_API causes crash with turbo >=1.11.0
chris-olszewski
added a commit
that referenced
this issue
Jan 6, 2024
### Description Fixes #6921 Setting `TURBO_API` env var to an empty string results in a panic: ``` [0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ TURBO_API= turbo_dev build --filter=docs --output-logs new-only • Packages in scope: docs • Running build in 1 packages • Remote caching enabled @turbo/workspaces:build: cache hit (outputs already on disk), suppressing logs 899e29b77bbaa6e9 docs:rss: cache hit (outputs already on disk), suppressing logs 9991721850b37e8a docs:schema: cache hit (outputs already on disk), suppressing logs 6048fc4f56a96905 @turbo/gen:build: cache hit (outputs already on disk), suppressing logs b8021fbfb2521690 docs:build: cache hit (outputs already on disk), suppressing logs 7f1f00f669ffea14 Tasks: 5 successful, 5 total Cached: 5 cached, 5 total Time: 495ms >>> FULL TURBO Oops! Turbo has crashed. A report has been written to /var/folders/3m/rxkycvgs5jgfvs0k9xcgp6km0000gn/T/report-9ec6c539-2888-430d-9f52-ecd91cf18bc9.toml Please open an issue at https://github.com/vercel/turbo/issues/new/choose and include this file WARNING failed to close spaces client ``` This is due to us expecting Go-like behavior where the empty string is the none value. We can achieve this behavior by mapping `Some("")` to `None`. I think this is a valid change for the various string type config options as the empty string is never a valid "present" value for these fields. ### Testing Instructions Added unit test to verify that we now treat empty set environment variables as `None` Quick spot check that this no longer crashes and uses the default API instead of `""`: ``` [0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ TURBO_API= turbo_dev build --filter=docs --output-logs new-only • Packages in scope: docs • Running build in 1 packages • Remote caching enabled @turbo/workspaces:build: cache hit (outputs already on disk), suppressing logs 899e29b77bbaa6e9 docs:schema: cache hit (outputs already on disk), suppressing logs 6048fc4f56a96905 @turbo/gen:build: cache hit (outputs already on disk), suppressing logs b8021fbfb2521690 docs:rss: cache hit, suppressing logs 9991721850b37e8a docs:build: cache hit (outputs already on disk), suppressing logs 7f1f00f669ffea14 Tasks: 5 successful, 5 total Cached: 5 cached, 5 total Time: 502ms >>> FULL TURBO Run: https://vercel.com/teams/vercel/repos/turbo-monorepo/runs/space_run_EfSNkNZDvtznuFbWhHGfbv1a ``` Closes TURBO-2004
chris-olszewski
added a commit
that referenced
this issue
Jan 9, 2024
### Description As reported in #6921 `turbo` is panicking when making HTTP requests. This PR doesn't fix the user issue, but it does avoid the panic and provide a better warning message to the end user. The panic can be caused in 2 ways: - Trying to retry a request that has a streaming body. This wasn't happening, but I changed the code so we at least don't panic if we accidentally call this function with a streaming body. - Constructing a `RequestBuilder` with an invalid url (see seanmonstar/reqwest#1943) will cause the `try_clone` method to return `None` causing us to panic. This can easily be done if a user passes an invalid API url. We mitigate this by now only constructing `RequestBuilder` with `Url` which has already been verified instead of `&str` which delays the verification and obscures the failure. Minor change is removing the `do_preflight` method from the client trait as it is an implementation detail and it reduced the amount of places that needed to get updated. ### Testing Instructions Create a bogus entry in `auth.json` e.g. ``` [0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ bat ~/Library/Application\ Support/turborepo/auth.json ───────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── │ File: /Users/olszewski/Library/Application Support/turborepo/auth.json ───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 1 │ { 2 │ "tokens": { 3 │ "junk-url": "junk-token" 4 │ } 5 │ } ───────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ``` On main verify that the current experience is far from ideal: <img width="956" alt="Screenshot 2024-01-05 at 1 26 09 PM" src="https://github.com/vercel/turbo/assets/4131117/f63c60f1-db0f-441f-a386-11e4078cd55c"> Build using these changes and verify that there's a better UX: <img width="1038" alt="Screenshot 2024-01-05 at 1 30 05 PM" src="https://github.com/vercel/turbo/assets/4131117/13fb3d21-bf48-44b9-9586-089d03afa8ee"> Closes TURBO-2002
Zertsov
pushed a commit
that referenced
this issue
Jan 10, 2024
### Description Fixes #6921 Setting `TURBO_API` env var to an empty string results in a panic: ``` [0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ TURBO_API= turbo_dev build --filter=docs --output-logs new-only • Packages in scope: docs • Running build in 1 packages • Remote caching enabled @turbo/workspaces:build: cache hit (outputs already on disk), suppressing logs 899e29b77bbaa6e9 docs:rss: cache hit (outputs already on disk), suppressing logs 9991721850b37e8a docs:schema: cache hit (outputs already on disk), suppressing logs 6048fc4f56a96905 @turbo/gen:build: cache hit (outputs already on disk), suppressing logs b8021fbfb2521690 docs:build: cache hit (outputs already on disk), suppressing logs 7f1f00f669ffea14 Tasks: 5 successful, 5 total Cached: 5 cached, 5 total Time: 495ms >>> FULL TURBO Oops! Turbo has crashed. A report has been written to /var/folders/3m/rxkycvgs5jgfvs0k9xcgp6km0000gn/T/report-9ec6c539-2888-430d-9f52-ecd91cf18bc9.toml Please open an issue at https://github.com/vercel/turbo/issues/new/choose and include this file WARNING failed to close spaces client ``` This is due to us expecting Go-like behavior where the empty string is the none value. We can achieve this behavior by mapping `Some("")` to `None`. I think this is a valid change for the various string type config options as the empty string is never a valid "present" value for these fields. ### Testing Instructions Added unit test to verify that we now treat empty set environment variables as `None` Quick spot check that this no longer crashes and uses the default API instead of `""`: ``` [0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ TURBO_API= turbo_dev build --filter=docs --output-logs new-only • Packages in scope: docs • Running build in 1 packages • Remote caching enabled @turbo/workspaces:build: cache hit (outputs already on disk), suppressing logs 899e29b77bbaa6e9 docs:schema: cache hit (outputs already on disk), suppressing logs 6048fc4f56a96905 @turbo/gen:build: cache hit (outputs already on disk), suppressing logs b8021fbfb2521690 docs:rss: cache hit, suppressing logs 9991721850b37e8a docs:build: cache hit (outputs already on disk), suppressing logs 7f1f00f669ffea14 Tasks: 5 successful, 5 total Cached: 5 cached, 5 total Time: 502ms >>> FULL TURBO Run: https://vercel.com/teams/vercel/repos/turbo-monorepo/runs/space_run_EfSNkNZDvtznuFbWhHGfbv1a ``` Closes TURBO-2004
Zertsov
pushed a commit
that referenced
this issue
Jan 10, 2024
### Description As reported in #6921 `turbo` is panicking when making HTTP requests. This PR doesn't fix the user issue, but it does avoid the panic and provide a better warning message to the end user. The panic can be caused in 2 ways: - Trying to retry a request that has a streaming body. This wasn't happening, but I changed the code so we at least don't panic if we accidentally call this function with a streaming body. - Constructing a `RequestBuilder` with an invalid url (see seanmonstar/reqwest#1943) will cause the `try_clone` method to return `None` causing us to panic. This can easily be done if a user passes an invalid API url. We mitigate this by now only constructing `RequestBuilder` with `Url` which has already been verified instead of `&str` which delays the verification and obscures the failure. Minor change is removing the `do_preflight` method from the client trait as it is an implementation detail and it reduced the amount of places that needed to get updated. ### Testing Instructions Create a bogus entry in `auth.json` e.g. ``` [0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ bat ~/Library/Application\ Support/turborepo/auth.json ───────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── │ File: /Users/olszewski/Library/Application Support/turborepo/auth.json ───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 1 │ { 2 │ "tokens": { 3 │ "junk-url": "junk-token" 4 │ } 5 │ } ───────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ``` On main verify that the current experience is far from ideal: <img width="956" alt="Screenshot 2024-01-05 at 1 26 09 PM" src="https://github.com/vercel/turbo/assets/4131117/f63c60f1-db0f-441f-a386-11e4078cd55c"> Build using these changes and verify that there's a better UX: <img width="1038" alt="Screenshot 2024-01-05 at 1 30 05 PM" src="https://github.com/vercel/turbo/assets/4131117/13fb3d21-bf48-44b9-9586-089d03afa8ee"> Closes TURBO-2002
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Verify canary release
Link to code that reproduces this issue
https://github.com/vercel/turbo/tree/main/examples/with-docker
What package manager are you using / does the bug impact?
Yarn v1
What operating system are you using?
Linux
Which canary version will you have in your reproduction?
1.11.3-canary.2
Describe the Bug
After upgrading Turbo to
v1.11.x, building in our Docker image is throwingPanicerrors. Check the logs below.Expected Behavior
Building in Docker should work same in v1.10.x and previous versions.
To Reproduce
Duplicate Docker example, change Node version to v20 and run build.
I have verified, that our issue occurs on any Node version, with
turbo>=1.11.0Contents of `/tmp/report-*.toml` files
Additional context
Building locally on my M1 Macbook sometime works and sometime throws same error. Nothing has been changed/libraries other than
turboversion.I though, that it is linked with #6715, but still does not work after merging.
The text was updated successfully, but these errors were encountered: