fix: gracefully fail if we can't create ~/.wrangler/reporting.toml
#578
Conversation
🦋 Changeset detectedLatest commit: d66b5ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/1973896955/npm-package-wrangler-578You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/578/npm-package-wrangler-578Or you can use npx https://prerelease-registry.developers.workers.dev/runs/1973896955/npm-package-wrangler-578 dev path/to/script.js |
threepointone
force-pushed
the
graceful-dot-wrangler-creation-failure
branch
from
9da4331 to
145c096
Compare
packages/wrangler/src/reporting.ts
Outdated
| { title: "No", value: false }, | ||
| ], | ||
| initial: 2, | ||
| }); | ||
|
|
||
| if (userInput.sentryDecision === "once") { | ||
| if (userInput.sentryDecision !== false) { |
There was a problem hiding this comment.
this felt like a bug, it wouldn't track the current error if we chose "Always"
There was a problem hiding this comment.
I don't think this was a bug.
If you chose Always then the error gets logged below on line 96 (original).
There was a problem hiding this comment.
oh huh I missed that, you're right
packages/wrangler/src/reporting.ts
Outdated
| ); | ||
| const reportingConfigLocation = path.join(homePath, "reporting.toml"); | ||
| try { | ||
| if (!fs.existsSync(reportingConfigLocation)) { |
There was a problem hiding this comment.
Why this if statement?
There was a problem hiding this comment.
eh I might have been testing something, I suppose it's unnecessary. removing.
packages/wrangler/src/reporting.ts
Outdated
| { title: "No", value: false }, | ||
| ], | ||
| initial: 2, | ||
| }); | ||
|
|
||
| if (userInput.sentryDecision === "once") { | ||
| if (userInput.sentryDecision !== false) { |
There was a problem hiding this comment.
I don't think this was a bug.
If you chose Always then the error gets logged below on line 96 (original).
packages/wrangler/src/reporting.ts
Outdated
| import os from "node:os"; | ||
| import path from "path/posix"; | ||
| import path from "node:path/posix"; |
There was a problem hiding this comment.
We should probably drop the posix here.
threepointone
force-pushed
the
graceful-dot-wrangler-creation-failure
branch
from
145c096 to
807e708
Compare
In some scenarios (CI/CD, docker, etc), we won't have write access to `~/.wrangler`. We already don't write a configuration file there if one passes a `CF_API_TOKEN`/`CLOUDFLARE_API_TOKEN` env var. This also adds a guard when writing the error reporting configuration file.
threepointone
force-pushed
the
graceful-dot-wrangler-creation-failure
branch
from
807e708 to
d66b5ba
Compare
| @@ -72,7 +80,7 @@ export async function reportError(err: Error, origin = "") { | |||
| message: "Would you like to submit a report when an error occurs?", | |||
| choices: [ | |||
| { title: "Always", value: true }, | |||
| { title: "Yes", value: "once" }, | |||
| { title: "Only this time", value: "once" }, | |||
There was a problem hiding this comment.
"Only this time" is a little misleading because, to me, it suggests that we will do it this once and then never ask again...
|
Realising we have to do something else - in non-TTY environments, we need to not ask this question at all, since there will be no human to answer it. |
|
Aye! Of course. |
In some scenarios (CI/CD, docker, etc), we won't have write access to
~/.wrangler. We already don't write a configuration file there if one passes aCF_API_TOKEN/CLOUDFLARE_API_TOKENenv var. This also adds a guard when writing the error reporting configuration file.