-
Notifications
You must be signed in to change notification settings - Fork 27k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Better dotenv error messages (vercel/turborepo#4464)
The issue message emitted by `TryDotenvProcessEnv` was pretty bad: `Execution of TryDotenvProcessEnv::read_all failed`. This error message comes from #3550, which attaches the calling function as the context for every failed turbo function. This PR accomplishes 2 main things: 1. Expose an explicit method determining whether a the `DotenvEnvProcess`'s prior or current env failed 2. Detects a current env failure and uses the error's root cause for the issue message. <img width="961" alt="Screen Shot 2023-04-04 at 7 29 52 PM" src="https://user-images.githubusercontent.com/112982/229944525-d39dbe87-778a-4421-9bc8-632924cd3782.png"> Fixes WEB-851 Tests: #47937
- Loading branch information
1 parent
3b97ebb
commit 19c98fd
Showing
11 changed files
with
126 additions
and
83 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,62 +1,61 @@ | ||
| use std::future::IntoFuture; | ||
|
|
||
| use anyhow::Result; | ||
| use turbo_tasks::primitives::{OptionStringVc, StringVc}; | ||
| use turbo_tasks::primitives::StringVc; | ||
| use turbo_tasks_env::{DotenvProcessEnvVc, EnvMapVc, ProcessEnv, ProcessEnvVc}; | ||
| use turbo_tasks_fs::FileSystemPathVc; | ||
|
|
||
| use crate::ProcessEnvIssue; | ||
|
|
||
| #[turbo_tasks::value] | ||
| pub struct TryDotenvProcessEnv { | ||
| dotenv: DotenvProcessEnvVc, | ||
| prior: ProcessEnvVc, | ||
| path: FileSystemPathVc, | ||
| } | ||
|
|
||
| impl TryDotenvProcessEnv { | ||
| async fn with_issue<T, V: Copy + IntoFuture<Output = Result<T>>>( | ||
| &self, | ||
| op: impl Fn(ProcessEnvVc) -> V, | ||
| ) -> Result<V> { | ||
| let r = op(DotenvProcessEnvVc::new(Some(self.prior), self.path).as_process_env()); | ||
| match r.await { | ||
| Ok(_) => Ok(r), | ||
| Err(e) => { | ||
| let r = op(self.prior); | ||
| // If the prior process env also reports an error we don't want to report our | ||
| // issue | ||
| r.await?; | ||
|
|
||
| ProcessEnvIssue { | ||
| path: self.path, | ||
| description: StringVc::cell(e.to_string()), | ||
| } | ||
| .cell() | ||
| .as_issue() | ||
| .emit(); | ||
| Ok(r) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[turbo_tasks::value_impl] | ||
| impl TryDotenvProcessEnvVc { | ||
| #[turbo_tasks::function] | ||
| pub fn new(prior: ProcessEnvVc, path: FileSystemPathVc) -> Self { | ||
| TryDotenvProcessEnv { prior, path }.cell() | ||
| let dotenv = DotenvProcessEnvVc::new(Some(prior), path); | ||
| TryDotenvProcessEnv { | ||
| dotenv, | ||
| prior, | ||
| path, | ||
| } | ||
| .cell() | ||
| } | ||
| } | ||
|
|
||
| #[turbo_tasks::value_impl] | ||
| impl ProcessEnv for TryDotenvProcessEnv { | ||
| #[turbo_tasks::function] | ||
| async fn read_all(&self) -> Result<EnvMapVc> { | ||
| self.with_issue(|e| e.read_all()).await | ||
| } | ||
| let dotenv = self.dotenv; | ||
| let prior = dotenv.read_prior(); | ||
|
|
||
| #[turbo_tasks::function] | ||
| async fn read(&self, name: &str) -> Result<OptionStringVc> { | ||
| self.with_issue(|e| e.read(name)).await | ||
| // Ensure prior succeeds. If it doesn't, then we don't want to attempt to read | ||
| // the dotenv file (and potentially emit an Issue), just trust that the prior | ||
| // will have emitted its own. | ||
| prior.await?; | ||
|
|
||
| let vars = dotenv.read_all_with_prior(prior); | ||
| match vars.await { | ||
| Ok(_) => Ok(vars), | ||
| Err(e) => { | ||
| // If parsing the dotenv file fails (but getting the prior value didn't), then | ||
| // we want to emit an Issue and fall back to the prior's read. | ||
| ProcessEnvIssue { | ||
| path: self.path, | ||
| // read_all_with_prior will wrap a current error with a context containing the | ||
| // failing file, which we don't really care about (we report the filepath as the | ||
| // Issue context, not the description). So extract the real error. | ||
| description: StringVc::cell(e.root_cause().to_string()), | ||
| } | ||
| .cell() | ||
| .as_issue() | ||
| .emit(); | ||
| Ok(prior) | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.