-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pretty-error page infrastructure #436
Merged
Merged
Conversation
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
|
mrbbot
force-pushed
the
tre-pretty-error
branch
from
November 18, 2022 10:41
08b0630
to
842fa53
Compare
penalosa
approved these changes
Nov 18, 2022
This adds Miniflare 2's pretty-error page powered by [Youch](https://github.com/poppinss/youch) to Miniflare 3. Unfortunately, due to a bug in `workerd`, errors thrown asynchronously by native APIs don't have `stack`s. This means we can't extract the `stack` trace from dispatching to the user worker by `try`/`catch`. As a stop-gap solution, if the `MF-Experimental-Error-Stack` header exists and is truthy on the response from the user worker, the body will be interpreted as a JSON-error of the form `{ message?: string, name?: string, stack?: string }`. `stack` will be source-mapped if possible. Another issue is that `workerd` gives all service-worker scripts the name "worker.js", so if multiple service-workers are defined, we can't identify which one threw. In this case, we don't display sources in the pretty-error page. Hopefully, we can fix both of these issues in `workerd`. We should be able to reuse most of this infrastructure with `try`/`catch`s too.
mrbbot
force-pushed
the
tre-pretty-error
branch
from
November 18, 2022 12:22
842fa53
to
2083935
Compare
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Nov 21, 2022
This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place.
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Nov 21, 2022
This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place.
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Nov 21, 2022
This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place.
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Nov 21, 2022
This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place.
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Nov 21, 2022
* Fix file-watching when using middleware with service workers Previously, when middleware was enabled, service worker user code was being bundled as part of the middleware facade application, not in the final bundling step with `watch` enabled. This meant changes to user code were not picked up. This change restructures the service worker middleware loader and bundling code as an IIFE, that gets injected into the final bundle. This also has the positive side effect that middleware internals aren't exposed to users. * Enable pretty-error page when using `--experimental-local` This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place.
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Nov 15, 2023
This change ensures Miniflare's pretty error page includes the URL and headers of the incoming request, rather than Miniflare's internal request from `workerd` to the loopback server. The request URL has been incorrect since the pretty error page was introduced into version 3 (cloudflare/miniflare#436). The request headers have been incorrect since (cloudflare/miniflare#681). This change also adds some tests to prevent this regressing again.
5 tasks
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Dec 8, 2023
This change ensures Miniflare's pretty error page includes the URL and headers of the incoming request, rather than Miniflare's internal request from `workerd` to the loopback server. The request URL has been incorrect since the pretty error page was introduced into version 3 (cloudflare/miniflare#436). The request headers have been incorrect since (cloudflare/miniflare#681). This change also adds some tests to prevent this regressing again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds Miniflare 2's pretty-error page powered by Youch to Miniflare 3.
Unfortunately, due to a bug in
workerd
, errors thrown asynchronously by native APIs don't havestack
s. This means we can't extract thestack
trace from dispatching to the user worker bytry
/catch
.As a stop-gap solution, if the
MF-Experimental-Error-Stack
header exists and is truthy on the response from the user worker, the body will be interpreted as a JSON-error of the form{ message?: string, name?: string, stack?: string }
.stack
will be source-mapped if possible.Another issue is that
workerd
gives all service-worker scripts the name "worker.js", so if multiple service-workers are defined, we can't identify which one threw. In this case, we don't display sources in the pretty-error page.Hopefully, we can fix both of these issues in
workerd
. We should be able to reuse most of this infrastructure withtry
/catch
s too.