Skip to content

Commit

Permalink
[wrangler] Changes for the next Miniflare version (#3951)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
mrbbot and jspspike authored Sep 19, 2023
1 parent 37f8731 commit e0850ad
Show file tree
Hide file tree
Showing 32 changed files with 1,188 additions and 851 deletions.
12 changes: 12 additions & 0 deletions .changeset/clever-bears-sip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"wrangler": minor
---

feat: add support for breakpoint debugging to `wrangler dev`'s `--remote` and `--no-bundle` modes

Previously, breakpoint debugging using Wrangler's DevTools was only supported
in local mode, when using Wrangler's built-in bundler. This change extends that
to remote development, and `--no-bundle`.

When using `--remote` and `--no-bundle` together, uncaught errors will now be
source-mapped when logged too.
27 changes: 27 additions & 0 deletions .changeset/twelve-numbers-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
"wrangler": minor
---

feat: add support for Visual Studio Code's built-in breakpoint debugger

Wrangler now supports breakpoint debugging with Visual Studio Code's debugger.
Create a `.vscode/launch.json` file with the following contents...

```json
{
"configurations": [
{
"name": "Wrangler",
"type": "node",
"request": "attach",
"port": 9229,
"cwd": "/",
"resolveSourceMapLocations": null,
"attachExistingChildren": false,
"autoAttachChildProcesses": false
}
]
}
```

...then run `wrangler dev`, and launch the configuration.
9 changes: 3 additions & 6 deletions fixtures/pages-workerjs-directory/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,9 @@ describe("Pages _worker.js/ directory", () => {
await stop();
}

expect(existsSync(join(tmpDir, "./v3/d1/D1"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/d1/elsewhere"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/kv/KV"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/kv/other_kv"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/r2/R2"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/r2/other_r2"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/d1"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/kv"))).toBeTruthy();
expect(existsSync(join(tmpDir, "./v3/r2"))).toBeTruthy();
});

it("should bundle", async ({ expect }) => {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"patchedDependencies": {
"ink@3.2.0": "patches/ink@3.2.0.patch",
"toucan-js@3.2.2": "patches/toucan-js@3.2.2.patch",
"@cloudflare/component-listbox@1.10.4": "patches/@cloudflare__component-listbox@1.10.4.patch"
"@cloudflare/component-listbox@1.10.6": "patches/@cloudflare__component-listbox@1.10.6.patch"
}
}
}
2 changes: 1 addition & 1 deletion packages/workers-playground/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"@cloudflare/component-code-block": "^4.1.2",
"@cloudflare/component-icon": "^11.5.1",
"@cloudflare/component-input": "^8.1.1",
"@cloudflare/component-listbox": "^1.10.4",
"@cloudflare/component-listbox": "^1.10.6",
"@cloudflare/component-loading": "^6.1.1",
"@cloudflare/component-textarea": "^4.1.1",
"@cloudflare/component-toast": "^5.1.0",
Expand Down
7 changes: 4 additions & 3 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,12 @@
"blake3-wasm": "^2.1.5",
"chokidar": "^3.5.3",
"esbuild": "0.17.19",
"miniflare": "3.20230904.0",
"miniflare": "3.20230918.0",
"nanoid": "^3.3.3",
"path-to-regexp": "^6.2.0",
"selfsigned": "^2.0.1",
"source-map": "^0.7.4",
"source-map": "0.6.1",
"source-map-support": "0.5.21",
"xxhash-wasm": "^1.0.1"
},
"devDependencies": {
Expand All @@ -122,7 +123,6 @@
"@cloudflare/workers-types": "^4.20230724.0",
"@iarna/toml": "^3.0.0",
"@microsoft/api-extractor": "^7.28.3",
"@types/better-sqlite3": "^7.6.0",
"@types/body-parser": "^1.19.2",
"@types/busboy": "^1.5.0",
"@types/command-exists": "^1.2.0",
Expand All @@ -136,6 +136,7 @@
"@types/react": "^17.0.37",
"@types/serve-static": "^1.13.10",
"@types/signal-exit": "^3.0.1",
"@types/source-map-support": "^0.5.7",
"@types/supports-color": "^8.1.1",
"@types/ws": "^8.5.3",
"@types/yargs": "^17.0.10",
Expand Down
14 changes: 12 additions & 2 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7945,7 +7945,12 @@ export default{
const bigModule = Buffer.alloc(10_000_000);
randomFillSync(bigModule);
await printBundleSize({ name: "index.js", content: "" }, [
{ name: "index.js", content: bigModule, type: "buffer" },
{
name: "index.js",
filePath: undefined,
content: bigModule,
type: "buffer",
},
]);

expect(std).toMatchInlineSnapshot(`
Expand All @@ -7968,7 +7973,12 @@ export default{
const bigModule = Buffer.alloc(10_000_000);
randomFillSync(bigModule);
await printBundleSize({ name: "index.js", content: "" }, [
{ name: "index.js", content: bigModule, type: "buffer" },
{
name: "index.js",
filePath: undefined,
content: bigModule,
type: "buffer",
},
]);

expect(std).toMatchInlineSnapshot(`
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/__tests__/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jest.mock("child_process", () => {
return {
__esModule: true,
...jest.requireActual("child_process"),
default: jest.requireActual("child_process"),
spawnSync: jest.fn().mockImplementation((binary, ...args) => {
if (binary === "cloudflared") return { error: true };
return jest.requireActual("child_process").spawnSync(binary, ...args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export async function createUploadWorkerBundleContents(
function createWorkerBundleFormData(workerBundle: BundleResult): FormData {
const mainModule = {
name: path.basename(workerBundle.resolvedEntryPointPath),
filePath: workerBundle.resolvedEntryPointPath,
content: readFileSync(workerBundle.resolvedEntryPointPath, {
encoding: "utf-8",
}),
Expand Down
42 changes: 15 additions & 27 deletions packages/wrangler/src/d1/execute.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import assert from "node:assert";
import { existsSync } from "node:fs";
import { mkdir } from "node:fs/promises";
import path from "node:path";
import chalk from "chalk";
import { Static, Text } from "ink";
import Table from "ink-table";
import { Miniflare } from "miniflare";
import React from "react";
import { fetchResult } from "../cfetch";
import { readConfig } from "../config";
Expand All @@ -29,7 +28,7 @@ import type {
StrictYargsOptionsToInterface,
} from "../yargs-types";
import type { Database } from "./types";
import type { D1SuccessResponse } from "miniflare";
import type { D1Result } from "@cloudflare/workers-types/experimental";

export type QueryResult = {
results: Record<string, string | number | boolean>[];
Expand Down Expand Up @@ -204,10 +203,8 @@ export async function executeSql({
? await executeLocally({
config,
name,
shouldPrompt,
queries,
persistTo,
json,
})
: await executeRemotely({
config,
Expand All @@ -224,17 +221,13 @@ export async function executeSql({
async function executeLocally({
config,
name,
shouldPrompt,
queries,
persistTo,
json,
}: {
config: Config;
name: string;
shouldPrompt: boolean | undefined;
queries: string[];
persistTo: string | undefined;
json: boolean | undefined;
}) {
const localDB = getDatabaseInfoFromConfig(config, name);
if (!localDB) {
Expand All @@ -245,30 +238,25 @@ async function executeLocally({

const id = localDB.previewDatabaseUuid ?? localDB.uuid;
const persistencePath = getLocalPersistencePath(persistTo, config.configPath);
const dbDir = path.join(persistencePath, "v3", "d1", id);
const dbPath = path.join(dbDir, `db.sqlite`);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const { D1Gateway, NoOpLog, createFileStorage } = require("miniflare");
const storage = createFileStorage(dbDir);
const d1Persist = path.join(persistencePath, "v3", "d1");

if (!existsSync(dbDir)) {
const ok =
json ||
!shouldPrompt ||
(await confirm(`About to create ${readableRelative(dbPath)}, ok?`));
if (!ok) return null;
await mkdir(dbDir, { recursive: true });
}
logger.log(`🌀 Loading ${id} from ${readableRelative(d1Persist)}`);

logger.log(`🌀 Loading DB at ${readableRelative(dbPath)}`);
const mf = new Miniflare({
modules: true,
script: "",
d1Persist,
d1Databases: { DATABASE: id },
});
const db = await mf.getD1Database("DATABASE");

const db = new D1Gateway(new NoOpLog(), storage);
let results: D1SuccessResponse | D1SuccessResponse[];
let results: D1Result<Record<string, string | number | boolean>>[];
try {
results = db.query(queries.map((query) => ({ sql: query })));
results = await db.batch(queries.map((query) => db.prepare(query)));
} catch (e: unknown) {
throw (e as { cause?: unknown })?.cause ?? e;
} finally {
await mf.dispose();
}
assert(Array.isArray(results));
return results.map<QueryResult>((result) => ({
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
// We want to know if the build is for development or publishing
// This could potentially cause issues as we no longer have identical behaviour between dev and deploy?
targetConsumer: "deploy",
local: false,
}
);

Expand Down Expand Up @@ -558,6 +559,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
if (assets.manifest) {
modules.push({
name: "__STATIC_CONTENT_MANIFEST",
filePath: undefined,
content: JSON.stringify(assets.manifest),
type: "text",
});
Expand All @@ -571,6 +573,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
name: scriptName,
main: {
name: path.basename(resolvedEntryPointPath),
filePath: resolvedEntryPointPath,
content: content,
type: bundleType,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/deployment-bundle/bundle-reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Metafile } from "esbuild";
const ONE_KIB_BYTES = 1024;
const ALLOWED_INITIAL_MAX = ONE_KIB_BYTES * 1024; // Current max is 1 MiB

async function getSize(modules: CfModule[]) {
async function getSize(modules: Pick<CfModule, "content">[]) {
const gzipSize = gzipSync(
await new Blob(modules.map((file) => file.content)).arrayBuffer()
).byteLength;
Expand Down
21 changes: 11 additions & 10 deletions packages/wrangler/src/deployment-bundle/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export async function bundleWorker(
disableModuleCollection?: boolean;
isOutfile?: boolean;
forPages?: boolean;
local: boolean;
}
): Promise<BundleResult> {
const {
Expand Down Expand Up @@ -175,12 +176,16 @@ export async function bundleWorker(
isOutfile,
forPages,
additionalModules = [],
local,
} = options;

// We create a temporary directory for any one-off files we
// need to create. This is separate from the main build
// directory (`destination`).
const tmpDir = await tmp.dir({ unsafeCleanup: true });
const unsafeTmpDir = await tmp.dir({ unsafeCleanup: true });
// Make sure we resolve all files relative to the actual temporary directory,
// without symlinks, otherwise `esbuild` will generate invalid source maps.
const tmpDirPath = fs.realpathSync(unsafeTmpDir.path);

const entryDirectory = path.dirname(entry.file);
let moduleCollector = createModuleCollector({
Expand Down Expand Up @@ -215,10 +220,10 @@ export async function bundleWorker(
// we need to extract that file to an accessible place before injecting
// it in, hence this code here.

const checkedFetchFileToInject = path.join(tmpDir.path, "checked-fetch.js");
const checkedFetchFileToInject = path.join(tmpDirPath, "checked-fetch.js");

if (checkFetch && !fs.existsSync(checkedFetchFileToInject)) {
fs.mkdirSync(tmpDir.path, {
fs.mkdirSync(tmpDirPath, {
recursive: true,
});
fs.writeFileSync(
Expand Down Expand Up @@ -253,7 +258,7 @@ export async function bundleWorker(
{
name: "miniflare3-json-error",
path: "templates/middleware/middleware-miniflare3-json-error.ts",
active: targetConsumer === "dev",
active: targetConsumer === "dev" && local,
},
{
name: "serve-static-assets",
Expand Down Expand Up @@ -320,7 +325,7 @@ export async function bundleWorker(
) {
inputEntry = await applyMiddlewareLoaderFacade(
entry,
tmpDir.path,
tmpDirPath,
activeMiddleware,
doBindings
);
Expand Down Expand Up @@ -450,7 +455,7 @@ export async function bundleWorker(
stop,
sourceMapPath,
sourceMapMetadata: {
tmpDir: tmpDir.path,
tmpDir: tmpDirPath,
entryDirectory: entry.directory,
},
moduleCollector,
Expand Down Expand Up @@ -482,10 +487,6 @@ async function applyMiddlewareLoaderFacade(
// and then we load the middleware - this insertion and loading is
// different for each format.

// Make sure we resolve all files relative to the actual temporary directory,
// otherwise we'll have issues with source maps
tmpDirPath = fs.realpathSync(tmpDirPath);

// We need to import each of the middlewares, so we need to generate a
// random, unique identifier that we can use for the import.
// Middlewares are required to be default exports so we can import to any name.
Expand Down
9 changes: 9 additions & 0 deletions packages/wrangler/src/deployment-bundle/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ export interface CfModule {
* './src/index.js'
*/
name: string;
/**
* The absolute path of the module on disk, or `undefined` if this is a
* virtual module. Used as the source URL for this module, so source maps are
* correctly resolved.
*
* @example
* '/path/to/src/index.js'
*/
filePath: string | undefined;
/**
* The module content, usually JavaScript or WASM code.
*
Expand Down
Loading

0 comments on commit e0850ad

Please sign in to comment.