[Miniflare 3] Improved source map handling and support for VSCode's debugger #681
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
commented
Sep 8, 2023
mrbbot
force-pushed
the
bcoll/tre-source-urls
branch
from
40cd2fe to
a26b0c9
Compare
RamIdeas
approved these changes
Sep 11, 2023
Previously, `workerd` didn't know the on-disk location of source files. This is required to resolve source maps. To support debugging, Miniflare rewrote `//# sourceMappingURL` comments to `http:` URLs hosted with the loopback server. This responded with transformed source maps using the correct source root. Whilst this worked with our patched DevTools and WebStorm, it didn't work with VSCode's built-in debugger. This change uses `//# sourceURL` comments to give the on-disk location to `workerd`. This massively simplifies lots of our source mapping code, and removes the need for the `SourceMapRegistry` entirely. It also allows us to give `workerd` paths for service worker scripts that aren't service names. `Debugger.scriptParsed` events emitted by `workerd`'s inspector server include these URLs, and resolve source map URLs relative to them.
Previously, we'd only log uncaught errors if we were returning the HTML pretty error page. Notably, this meant we wouldn't log errors if sending requests with `curl`. This change ensures we always send a request to Miniflare's loopback server, logging the error. This also means plain-text error responses are now source mapped too.
This change massively improves the debuggability of Miniflare's internal Durable Objects. Uncaught errors throws by Durable Objects will now be source-mapped and logged. `//# sourceURL` comments are also added to all Miniflare internal scripts, allowing for breakpoint debugging.
mrbbot
force-pushed
the
bcoll/tre-source-urls
branch
from
a26b0c9 to
a2e1776
Compare
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Sep 14, 2023
Previously, Wrangler would respond to `Network.loadNetworkResource` requests with the bundled `esbuild` source map in `--remote` mode. In local mode, Wrangler relied on Miniflare rewriting source mapping URLs to its loopback server. Unfortunately, this breaks breakpoint debugging in VSCode, so was removed in cloudflare/miniflare#681 in favour of a `//# sourceURL`-based approach. With the previous changes to include `//# sourceURL` comments in remote mode too, this means all source mapping URLs from Miniflare and remote dev will now have `file:` protocols. These cannot be fetched from our DevTools hosted on a `https:` origin. IDEs like VSCode and WebStorm expect these though. To fix hosted DevTools, this PR rewrites `Debugger.scriptParsed` events to include `worker:`-scheme URLs, only if the connected inspector client is DevTools. Wrangler will then respond to `Network.loadNetworkResource` with the source map. As noted above, Wrangler used to only respond with the source map from the internal `esbuild` bundle step. When using `--no-bundle`, users may bring their own source map**s**. Previously, these weren't served, preventing these users from using DevTools. With the new `//# sourceURL` comments, we're able to work out which source map corresponds to which source file, meaning Wrangler can now serve multiple source maps. These source maps may not include `sourcesContent` fields. In this case, DevTools sends additional `Network.loadNetworkResource` requests for sources, which Wrangler now responds to too.
mrbbot
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Sep 14, 2023
Previously, Wrangler would respond to `Network.loadNetworkResource` requests with the bundled `esbuild` source map in `--remote` mode. In local mode, Wrangler relied on Miniflare rewriting source mapping URLs to its loopback server. Unfortunately, this breaks breakpoint debugging in VSCode, so was removed in cloudflare/miniflare#681 in favour of a `//# sourceURL`-based approach. With the previous changes to include `//# sourceURL` comments in remote mode too, this means all source mapping URLs from Miniflare and remote dev will now have `file:` protocols. These cannot be fetched from our DevTools hosted on a `https:` origin. IDEs like VSCode and WebStorm expect these though. To fix hosted DevTools, this PR rewrites `Debugger.scriptParsed` events to include `worker:`-scheme URLs, only if the connected inspector client is DevTools. Wrangler will then respond to `Network.loadNetworkResource` with the source map. As noted above, Wrangler used to only respond with the source map from the internal `esbuild` bundle step. When using `--no-bundle`, users may bring their own source map**s**. Previously, these weren't served, preventing these users from using DevTools. With the new `//# sourceURL` comments, we're able to work out which source map corresponds to which source file, meaning Wrangler can now serve multiple source maps. These source maps may not include `sourcesContent` fields. In this case, DevTools sends additional `Network.loadNetworkResource` requests for sources, which Wrangler now responds to too.
2 tasks
jspspike
pushed a commit
to cloudflare/workers-sdk
that referenced
this pull request
Sep 19, 2023
Previously, Wrangler would respond to `Network.loadNetworkResource` requests with the bundled `esbuild` source map in `--remote` mode. In local mode, Wrangler relied on Miniflare rewriting source mapping URLs to its loopback server. Unfortunately, this breaks breakpoint debugging in VSCode, so was removed in cloudflare/miniflare#681 in favour of a `//# sourceURL`-based approach. With the previous changes to include `//# sourceURL` comments in remote mode too, this means all source mapping URLs from Miniflare and remote dev will now have `file:` protocols. These cannot be fetched from our DevTools hosted on a `https:` origin. IDEs like VSCode and WebStorm expect these though. To fix hosted DevTools, this PR rewrites `Debugger.scriptParsed` events to include `worker:`-scheme URLs, only if the connected inspector client is DevTools. Wrangler will then respond to `Network.loadNetworkResource` with the source map. As noted above, Wrangler used to only respond with the source map from the internal `esbuild` bundle step. When using `--no-bundle`, users may bring their own source map**s**. Previously, these weren't served, preventing these users from using DevTools. With the new `//# sourceURL` comments, we're able to work out which source map corresponds to which source file, meaning Wrangler can now serve multiple source maps. These source maps may not include `sourcesContent` fields. In this case, DevTools sends additional `Network.loadNetworkResource` requests for sources, which Wrangler now responds to too.
jspspike
added a commit
to cloudflare/workers-sdk
that referenced
this pull request
Sep 19, 2023
* Use Miniflare's magic proxy for `wrangler kv --local` commands `miniflare` no longer provides Node APIs for accessing its persistent storage directly, as simulators are now implemented as Workers. This change updates the `wrangler kv --local` commands to use `Miniflare#getKVNamespace()` instead, proxying through the Worker simulator. * Use Miniflare's magic proxy for `wrangler r2 --local` commands As with KV, this change updates the `wrangler kv --local` commands to use `Miniflare#getR2Bucket()` instead, proxying through the Worker simulator. * Use Miniflare's magic proxy for `wrangler d1 --local` commands As with KV and R2, this change updates the `wrangler d1 --local` commands to use `Miniflare#getD1Database()` instead, proxying through the Worker simulator. This removes Wrangler's dependency on `better-sqlite3`, meaning we no longer have any native dependencies. * Update `pages-workerjs-directory` tests for new persistence layout Namespace IDs are now encoded into Durable Object IDs, so we cannot check for their existence directly. * Include `//# sourceURL` comments in remote mode Adds `//# sourceURL` comments to source files in `--remote` dev so V8 knows where source files are on disk. This URL is returned in `Debugger.scriptParsed` events, ensuring inspector clients resolve source mapping URLs correctly. It also appears in stack traces, allowing users to click through to where errors are thrown, and making it easier for us to source map them. Note, Miniflare already includes similar code, so we only need to do this in remote mode. This is a pre-requisite to enabling breakpoint debugging in remote. * Only enable Miniflare's JSON error middleware in local mode This middleware returns a machine-readable JSON response for uncaught errors. This is only intended for Miniflare's pretty error page and shouldn't be used in remote mode. We may want to add a pretty error page to remote mode eventually, but that should be a separate change. * Use `realpath`ed temporary directories to generate valid source maps On macOS, `os.tmpdir()` returns a symlink. This causes `esbuild` to generate invalid source maps, as the number of `../` in relative paths changes depending on whether you evaluate symlinks. We previously fixed a similar issue for the middleware loader (https://github.com/cloudflare/workers-sdk/pull/2249/files#diff-17e01c57aa611bb9e0b392a15fd63b5d18602e3a6c9a97c4a34e891bbfdcb7f3R496-R498), but the issue is more widespread than that. This change ensures all temporary directories are `realpath`ed and symlink free. * Serve source maps and source files from inspector proxy Previously, Wrangler would respond to `Network.loadNetworkResource` requests with the bundled `esbuild` source map in `--remote` mode. In local mode, Wrangler relied on Miniflare rewriting source mapping URLs to its loopback server. Unfortunately, this breaks breakpoint debugging in VSCode, so was removed in cloudflare/miniflare#681 in favour of a `//# sourceURL`-based approach. With the previous changes to include `//# sourceURL` comments in remote mode too, this means all source mapping URLs from Miniflare and remote dev will now have `file:` protocols. These cannot be fetched from our DevTools hosted on a `https:` origin. IDEs like VSCode and WebStorm expect these though. To fix hosted DevTools, this PR rewrites `Debugger.scriptParsed` events to include `worker:`-scheme URLs, only if the connected inspector client is DevTools. Wrangler will then respond to `Network.loadNetworkResource` with the source map. As noted above, Wrangler used to only respond with the source map from the internal `esbuild` bundle step. When using `--no-bundle`, users may bring their own source map**s**. Previously, these weren't served, preventing these users from using DevTools. With the new `//# sourceURL` comments, we're able to work out which source map corresponds to which source file, meaning Wrangler can now serve multiple source maps. These source maps may not include `sourcesContent` fields. In this case, DevTools sends additional `Network.loadNetworkResource` requests for sources, which Wrangler now responds to too. * Source map `Runtime.exceptionThrown` events with multiple sources When `Runtime.exceptionThrown` events are dispatched by V8, Wrangler previously only used the source map produced by `esbuild` to source map the stack trace. In `--no-bundle` mode, it's possible there might be multiple user provided source maps needed to source map the stack trace. This change switches to using the `source-map-support` package to source map stack traces. Miniflare uses this already, so we're not adding a new dependency. This relies on file URLs in stack traces, something we now have from adding `//# sourceURL` comments. * Enable DevTools breakpoint debugging in remote mode Now that we're adding `//# sourceURL` comments in remote mode, breakpoint debugging just works in remote mode, including in IDEs. This change sets the `?debugger=true` query param when opening DevTools regardless of the runtime in-use. * Bump `miniflare` version to `3.20230918` --------- Co-authored-by: jjohnson <jjohnson@cloudflare.com>
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.
Hey! 👋 This PR includes a bunch of changes aimed at improving debugging support. Notably, this PR adds support for VSCode's built-in debugger, and step-through debugging of Miniflare's internal Workers. See the individual commit descriptions for more details.