fix: don't crash when browser windows don't open #415
Conversation
🦋 Changeset detectedLatest commit: 0261979 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 |
|
wrangler prerelease is available for testing: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/1817914398/wrangler |
There was a problem hiding this comment.
Could we just move this into a helper function? It would help keep the call-sites cleaner and would also encourage others to do the right thing when they need to open a thingy.
It also would help, later, when writing tests, since it would be easier to mock out.
Would be only one place for a Sentry capture as well and cover way more ground. |
|
Sorry if I'm missing something, but I don't think this is related to sentry? Do we suppose we'll want to capture failures to open these windows? I'd think not. It usually means that they're using wrangler inside something like codespaces, where the failure is expected, we just don't want the process to crash. |
We open browser windows for a few things; during `wrangler dev`, and logging in. There are environments where this doesn't work as expected (like codespaces, stackblitz, etc). This fix simply logs an error instead of breaking the flow. This is the same fix as #263, now applied to the rest of wrangler.
threepointone
force-pushed
the
fix-url-open
branch
from
217fa0e to
0261979
Compare
| const childProcess = await open(`http://localhost:${port}/`); | ||
| // fail silently if the open command doesn't work (e.g. in GitHub Codespaces) | ||
| childProcess.on("error", (_err) => {}); | ||
| await openInBrowser(`http://localhost:${port}/`); |
There was a problem hiding this comment.
@GregBrimble ^ I also replaced pages' usage of open(), fyi.
|
Hey @threepointone, there's a typo in the |
|
lol dammit, thanks @mrbbot |
We open browser windows for a few things; during
wrangler dev, and logging in. There are environments where this doesn't work as expected (like codespaces, stackblitz, etc). This fix simply logs an error instead of breaking the flow. This is the same fix as #263, now applied to the rest of wrangler.