feat: recommend node compat on builtin use #1027
Conversation
🦋 Changeset detectedLatest commit: 5c4145f 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/2333487697/npm-package-wrangler-1027You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1027/npm-package-wrangler-1027Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2333487697/npm-package-wrangler-1027 dev path/to/script.js |
|
The windows CI failure is because of #977, investigating that separately |
Co-authored-by: Sunil Pai <[email protected]>
| ), | ||
| }, | ||
| () => { | ||
| throw new Error( |
There was a problem hiding this comment.
Should use the logger Error utility
logger.error()
There was a problem hiding this comment.
We went it to throw an error though, not just log
There was a problem hiding this comment.
throw the logger.error or we should add the ability to pass a option that has the logger throw Error seems like it should somehow be consistent and utilize the utility, I will unblock the PR regardless
There was a problem hiding this comment.
I don't understand, the logger only logs, but we want this to be an exception. We log the error to the terminal the error when it gets caught anyway. Where are we missing consistency?
There was a problem hiding this comment.
I don't understand, the logger only logs, but we want this to be an exception. We log the error to the terminal the error when it gets caught anyway. Where are we missing consistency?
logger.error seems redundant then, you can also throw anything. Consistency is lost in how we console out log errors vs exceptions errors, one has a custom format the other is default system format. Not causing a SIGTERM for some console errors makes sense I suppose like in validation so you can collect all the errors before SIGTERM, I just don't understand why they need to look different too?
There was a problem hiding this comment.
Wait maybe I missed this,
We log the error to the terminal the error when it gets caught anyway.
All Errors are now passed into the logger.error?
If that is the case ignore everything I said because that is what I was implying we do, one way or another.
EDIT: confirmed in exception handlers the error is passed into logger.error
There was a problem hiding this comment.
There was a problem hiding this comment.
@rozenmd could you add a small screenshot of what the error looks like?
There was a problem hiding this comment.
@threepointone: (using the latest branch preview in this PR)
TODO:
Closes #989