diff --git a/.changeset/silver-sheep-melt.md b/.changeset/silver-sheep-melt.md index a008ca152c..e5916b3532 100644 --- a/.changeset/silver-sheep-melt.md +++ b/.changeset/silver-sheep-melt.md @@ -9,7 +9,9 @@ Enable debugger connections by passing `--debug` flag to the `h2 dev` command: You can then connect to the port `9229` (configurable with the new `--inspector-port` flag) to start step debugging. -For example, in Chrome you can go to `chrome://inspect` and make sure the inspector port is added to the network targets. In VSCode, you can add the following to your `.vscode/launch.json`: +When using `--worker-unstable`, an improved version of the DevTools will be available in `localhost:9229`. Otherwise, in Chrome you can go to `chrome://inspect` to open the DevTools -- make sure the inspector port is added to the network targets. + +Alternatively, in VSCode, you can add the following to your `.vscode/launch.json`: ``` { diff --git a/packages/cli/src/lib/mini-oxygen/workerd-inspector-logs.ts b/packages/cli/src/lib/mini-oxygen/workerd-inspector-logs.ts index 0f397efa1b..793d26efaf 100644 --- a/packages/cli/src/lib/mini-oxygen/workerd-inspector-logs.ts +++ b/packages/cli/src/lib/mini-oxygen/workerd-inspector-logs.ts @@ -1,8 +1,90 @@ import {type Protocol} from 'devtools-protocol'; -import {type ErrorProperties} from './workerd-inspector.js'; +import type { + InspectorConnection, + ErrorProperties, + MessageData, +} from './workerd-inspector.js'; import {SourceMapConsumer} from 'source-map'; import {parse as parseStackTrace} from 'stack-trace'; +export function addInspectorConsoleLogger(inspector: InspectorConnection) { + inspector.ws.addEventListener('message', async (event) => { + if (typeof event.data !== 'string') { + // We should never get here, but who know... + console.error('Unrecognised devtools event:', event); + return; + } + + const evt = JSON.parse(event.data) as MessageData; + inspector.cleanupMessageQueue(evt); + + if (evt.method === 'Runtime.consoleAPICalled') { + await logConsoleMessage(evt.params, inspector); + } else if (evt.method === 'Runtime.exceptionThrown') { + console.error( + await createErrorFromException(evt.params.exceptionDetails, inspector), + ); + } + }); +} + +export async function createErrorFromException( + exceptionDetails: Protocol.Runtime.ExceptionDetails, + inspector: InspectorConnection, +) { + const errorProperties: ErrorProperties = {}; + + const sourceMapConsumer = await inspector.getSourceMapConsumer(); + if (sourceMapConsumer !== undefined) { + // Create the lines for the exception details log + const message = exceptionDetails.exception?.description?.split('\n')[0]; + const stack = exceptionDetails.stackTrace?.callFrames; + const formatted = formatStructuredError(sourceMapConsumer, message, stack); + + errorProperties.message = exceptionDetails.text; + errorProperties.stack = formatted; + } else { + errorProperties.message = + exceptionDetails.text + + ' ' + + (exceptionDetails.exception?.description ?? ''); + } + + return inspector.reconstructError( + errorProperties, + exceptionDetails.exception, + ); +} + +export async function createErrorFromLog( + ro: Protocol.Runtime.RemoteObject, + inspector: InspectorConnection, +) { + if (ro.subtype !== 'error' || ro.preview?.subtype !== 'error') { + throw new Error('Not an error object'); + } + + const errorProperties = { + message: + ro.preview.description + ?.split('\n') + .filter((line) => !/^\s+at\s/.test(line)) + .join('\n') ?? + ro.preview.properties.find(({name}) => name === 'message')?.value ?? + '', + stack: + ro.preview.description ?? + ro.description ?? + ro.preview.properties.find(({name}) => name === 'stack')?.value, + cause: ro.preview.properties.find(({name}) => name === 'cause') + ?.value as unknown, + } satisfies ErrorProperties; + + // Even though we have gathered all the properties, they are likely + // truncated so we need to fetch their full version. + return inspector.reconstructError(errorProperties, ro); +} + /** * This function converts a message serialised as a devtools event * into arguments suitable to be called by a console method, and @@ -11,7 +93,7 @@ import {parse as parseStackTrace} from 'stack-trace'; * directly in the terminal. */ -export const mapConsoleAPIMessageTypeToConsoleMethod: { +const mapConsoleAPIMessageTypeToConsoleMethod: { [key in Protocol.Runtime.ConsoleAPICalledEvent['type']]: Exclude< keyof Console, 'Console' @@ -37,12 +119,9 @@ export const mapConsoleAPIMessageTypeToConsoleMethod: { endGroup: 'groupEnd', }; -export async function logConsoleMessage( +async function logConsoleMessage( evt: Protocol.Runtime.ConsoleAPICalledEvent, - reconstructError: ( - initialProperties: ErrorProperties, - ro: Protocol.Runtime.RemoteObject, - ) => Promise, + inspector: InspectorConnection, ) { const args: Array = []; for (const ro of evt.args) { @@ -141,28 +220,7 @@ export async function logConsoleMessage( case 'wasmvalue': break; case 'error': - const errorProperties = { - message: - ro.preview.description - ?.split('\n') - .filter((line) => !/^\s+at\s/.test(line)) - .join('\n') ?? - ro.preview.properties.find(({name}) => name === 'message') - ?.value ?? - '', - stack: - ro.preview.description ?? - ro.description ?? - ro.preview.properties.find(({name}) => name === 'stack') - ?.value, - cause: ro.preview.properties.find(({name}) => name === 'cause') - ?.value as unknown, - }; - - // Even though we have gathered all the properties, they are likely - // truncated so we need to fetch their full version. - const error = await reconstructError(errorProperties, ro); - + const error = await createErrorFromLog(ro, inspector); // Replace its description in args args.splice(-1, 1, error); diff --git a/packages/cli/src/lib/mini-oxygen/workerd-inspector-proxy.ts b/packages/cli/src/lib/mini-oxygen/workerd-inspector-proxy.ts index fdbbef0458..d2d0e3010c 100644 --- a/packages/cli/src/lib/mini-oxygen/workerd-inspector-proxy.ts +++ b/packages/cli/src/lib/mini-oxygen/workerd-inspector-proxy.ts @@ -6,13 +6,24 @@ import { type ServerResponse, } from 'node:http'; import {WebSocketServer, type WebSocket, type MessageEvent} from 'ws'; -import {type Protocol} from 'devtools-protocol'; -import {type InspectorWebSocketTarget} from './workerd-inspector.js'; +import type {Protocol} from 'devtools-protocol'; +import type { + MessageData, + InspectorWebSocketTarget, + InspectorConnection, +} from './workerd-inspector.js'; +import {request} from 'undici'; + +const CFW_DEVTOOLS = 'https://devtools.devprod.cloudflare.dev'; +const H2_FAVICON_URL = + 'https://cdn.shopify.com/s/files/1/0598/4822/8886/files/favicon.svg'; + +export type InspectorProxy = ReturnType; export function createInspectorProxy( port: number, sourceFilePath: string, - inspectorConnection?: {ws?: WebSocket}, + newInspectorConnection: InspectorConnection, ) { /** * A unique identifier for this debugging session. @@ -23,17 +34,20 @@ export function createInspectorProxy( */ let debuggerWs: WebSocket | undefined = undefined; /** - * WebSocket connection to the Workerd inspector. + * Workerd inspector connection. */ - let inspectorWs: WebSocket | undefined = inspectorConnection?.ws; + let inspector = newInspectorConnection; /** * Whether the connected debugger is running in the browser (e.g. DevTools). */ let isDevToolsInBrowser = false; + const sourceMapPathname = '/__index.js.map'; + const sourceMapURL = `http://localhost:${port}${sourceMapPathname}`; + const server = createServer((req: IncomingMessage, res: ServerResponse) => { // Remove query params. E.g. `/json/list?for_tab` - const url = req.url?.split('?')[0] ?? ''; + const [url = '/', queryString = ''] = req.url?.split('?') || []; switch (url) { // We implement a couple of well known end points @@ -62,24 +76,67 @@ export function createInspectorProxy( devtoolsFrontendUrlCompat, // Below are fields that are visible in the DevTools UI. title: 'Hydrogen / Oxygen Worker', - faviconUrl: - 'https://cdn.shopify.com/s/files/1/0598/4822/8886/files/favicon.svg', - url: - 'https://' + - (inspectorWs ? new URL(inspectorWs.url).host : localHost), + faviconUrl: H2_FAVICON_URL, + url: 'https://' + new URL(inspector.ws.url).host, } satisfies InspectorWebSocketTarget, ]), ); } return; - case '/__index.js.map': + case sourceMapPathname: // Handle proxied sourcemaps res.setHeader('Content-Type', 'text/plain'); res.setHeader('Cache-Control', 'no-store'); - res.setHeader('Access-Control-Allow-Origin', 'devtools://devtools'); + res.setHeader( + 'Access-Control-Allow-Origin', + req.headers.origin ?? 'devtools://devtools', + ); + res.end(readFileSync(sourceFilePath + '.map', 'utf-8')); break; + case '/favicon.ico': + proxyHttp(H2_FAVICON_URL, req.headers, res); + break; + case '/': + if (!queryString) { + // Redirect to the DevTools UI with proper query params. + res.statusCode = 302; + res.setHeader( + 'Location', + `/?experiments=true&v8only=true&debugger=true&ws=localhost:${port}/ws`, + ); + res.end(); + } else { + // Proxy CFW DevTools UI. + proxyHttp( + CFW_DEVTOOLS + '/js_app', + req.headers, + res, + (content) => + // HTML from DevTools comes without closing and tags. + // The browser closes them automatically, then modifies the DOM with JS. + // This adds a loading indicator before the JS kicks in and modifies the DOM. + content + + '
Loading DevTools...
' + + '', + ); + } + break; default: + if ( + url === '/panels/sources/sources-meta.js' || + (url.startsWith('/core/i18n/locales/') && url.endsWith('.json')) + ) { + // Replace tab names in the sources section. Locales might + // overwrite the original name, so we modify them too. + proxyHttp(CFW_DEVTOOLS + url, req.headers, res, (content) => + content.replace(/['"]Cloudflare['"]/g, '"Hydrogen"'), + ); + } else { + // Proxy all other assets to the CFW DevTools CDN. + proxyHttp(CFW_DEVTOOLS + url, req.headers, res); + } + break; } }); @@ -104,7 +161,7 @@ export function createInspectorProxy( } else { // Ensure debugger is restarted in workerd before connecting // a new client to receive `Debugger.scriptParsed` events. - inspectorWs?.send( + inspector.ws.send( JSON.stringify({id: 100_000_000, method: 'Debugger.disable'}), ); @@ -126,10 +183,10 @@ export function createInspectorProxy( } }); - if (inspectorWs) onInspectorConnection(); + if (inspector.ws) onInspectorConnection(); function onInspectorConnection() { - inspectorWs?.addEventListener('message', sendMessageToDebugger); + inspector.ws.addEventListener('message', sendMessageToDebugger); // In case this is a DevTools connection, send a warning // message to the console to inform about reconnection. @@ -156,25 +213,12 @@ export function createInspectorProxy( } function sendMessageToInspector(event: MessageEvent) { - inspectorWs?.send(event.data); + inspector.ws.send(event.data); } function sendMessageToDebugger(event: MessageEvent) { - // Intercept Debugger.scriptParsed responses to inject URL schemes - // so that DevTools can fetch source maps from the proxy server. - // This is only required when opening DevTools in the browser. if (isDevToolsInBrowser) { - const message = JSON.parse(event.data as string); - if ( - message.method === 'Debugger.scriptParsed' && - message.params.sourceMapURL === 'index.js.map' - ) { - // The browser can't download source maps from file:// URLs due to security restrictions. - // Force the DevTools to fetch the source map using http:// instead of file:// - // This endpoint is handled in our proxy server above. - message.params.sourceMapURL = `http://localhost:${port}/__index.js.map`; - event = {...event, data: JSON.stringify(message)}; - } + event = enhanceDevToolsEvent(event, sourceMapURL); } if (debuggerWs) { @@ -185,9 +229,72 @@ export function createInspectorProxy( } return { - updateInspectorConnection(newConnection?: {ws?: WebSocket}) { - inspectorWs = newConnection?.ws; + updateInspectorConnection(newConnection: InspectorConnection) { + inspector = newConnection; onInspectorConnection(); }, }; } + +function enhanceDevToolsEvent(event: MessageEvent, sourceMapUrl: string) { + const message = JSON.parse(event.data as string) as MessageData; + + if (message.method === 'Debugger.scriptParsed') { + // Intercept Debugger.scriptParsed responses to inject URL schemes + // so that DevTools can fetch source maps from the proxy server. + // This is only required when opening DevTools in the browser. + if (message.params.sourceMapURL === 'index.js.map') { + // The browser can't download source maps from file:// URLs due to security restrictions. + // Force the DevTools to fetch the source map using http:// instead of file:// + // This endpoint is handled in our proxy server above. + message.params.sourceMapURL = sourceMapUrl; + } + } + + return {...event, data: JSON.stringify(message)}; +} + +function proxyHttp( + url: string, + originalHeaders: IncomingMessage['headers'], + nodeResponse: ServerResponse, + contentReplacer?: (content: string) => string, +) { + const headers = Object.fromEntries(Object.entries(originalHeaders)); + delete headers['host']; + delete headers['cookie']; + // If the response is going to be awaited and modified, + // we can't ask for an encoded response (we can't decode it here). + if (contentReplacer) delete headers['accept-encoding']; + + // Use `request` instead of `fetch` to avoid decompressing + // the response body. We want to forward the raw response. + // https://github.com/nodejs/undici/issues/1462 + return request(url, {responseHeader: 'raw', headers}) + .then((response) => { + nodeResponse.statusCode = response.statusCode; + if (nodeResponse.statusCode === 404) { + return nodeResponse.end('Not found'); + } + + Object.entries(response.headers).forEach(([key, value]) => { + if (value) { + nodeResponse.setHeader(key, value); + } + }); + + if (contentReplacer) { + return response.body + ?.text() + .then(contentReplacer) + .then(nodeResponse.end.bind(nodeResponse)); + } + + return response.body.pipe(nodeResponse); + }) + .catch((err) => { + console.error(err); + nodeResponse.statusCode = 500; + nodeResponse.end('Internal error'); + }); +} diff --git a/packages/cli/src/lib/mini-oxygen/workerd-inspector.ts b/packages/cli/src/lib/mini-oxygen/workerd-inspector.ts index d0a5b5a263..1d41d6ea5a 100644 --- a/packages/cli/src/lib/mini-oxygen/workerd-inspector.ts +++ b/packages/cli/src/lib/mini-oxygen/workerd-inspector.ts @@ -7,13 +7,17 @@ import {dirname} from 'node:path'; import {readFile} from 'node:fs/promises'; import {fetch} from '@shopify/cli-kit/node/http'; import {SourceMapConsumer} from 'source-map'; -import {WebSocket, type MessageEvent} from 'ws'; +import {WebSocket} from 'ws'; import {type Protocol} from 'devtools-protocol'; import { + addInspectorConsoleLogger, formatStack, - formatStructuredError, - logConsoleMessage, } from './workerd-inspector-logs.js'; +import {AbortError} from '@shopify/cli-kit/node/error'; +import { + createInspectorProxy, + type InspectorProxy, +} from './workerd-inspector-proxy.js'; // https://chromedevtools.github.io/devtools-protocol/#endpoints export interface InspectorWebSocketTarget { @@ -28,7 +32,67 @@ export interface InspectorWebSocketTarget { url: string; } -export async function findInspectorUrl(inspectorPort: number) { +export type MessageData = {id: number; result: unknown} & ( + | { + method: 'Debugger.scriptParsed'; + params: Protocol.Debugger.ScriptParsedEvent; + } + | { + method: 'Runtime.consoleAPICalled'; + params: Protocol.Runtime.ConsoleAPICalledEvent; + } + | { + method: 'Runtime.exceptionThrown'; + params: Protocol.Runtime.ExceptionThrownEvent; + } +); + +export interface ErrorProperties { + message?: string; + cause?: unknown; + stack?: string; +} + +export function createInspectorConnector(options: { + privateInspectorPort: number; + publicInspectorPort: number; + absoluteBundlePath: string; + sourceMapPath: string; + debug: boolean; +}) { + let inspectorUrl: string | undefined; + let inspectorConnection: InspectorConnection | undefined; + let inspectorProxy: InspectorProxy | undefined; + + return async (onBeforeConnect?: () => void | Promise) => { + inspectorConnection?.close(); + + inspectorUrl ??= await findInspectorUrl(options.privateInspectorPort); + + await onBeforeConnect?.(); + + inspectorConnection = connectToInspector({ + inspectorUrl, + sourceMapPath: options.sourceMapPath, + }); + + addInspectorConsoleLogger(inspectorConnection); + + if (options.debug) { + if (inspectorProxy) { + inspectorProxy.updateInspectorConnection(inspectorConnection); + } else { + inspectorProxy = createInspectorProxy( + options.publicInspectorPort, + options.absoluteBundlePath, + inspectorConnection, + ); + } + } + }; +} + +async function findInspectorUrl(inspectorPort: number) { try { // Fetch the inspector JSON response from the DevTools Inspector protocol const jsonUrl = `http://127.0.0.1:${inspectorPort}/json`; @@ -36,13 +100,28 @@ export async function findInspectorUrl(inspectorPort: number) { await fetch(jsonUrl) ).json()) as InspectorWebSocketTarget[]; - return body?.find(({id}) => id === 'core:user:hydrogen') - ?.webSocketDebuggerUrl; + const url = body?.find( + ({id}) => id === 'core:user:hydrogen', + )?.webSocketDebuggerUrl; + + if (!url) { + throw new Error('Unable to find inspector URL'); + } + + return url; } catch (error: unknown) { - console.error('Error attempting to retrieve debugger URL:', error); + const abortError = new AbortError( + 'Unable to connect to Worker inspector', + `Please report this issue. ${(error as Error).stack}`, + ); + + abortError.stack = (error as Error).stack; + throw abortError; } } +export type InspectorConnection = ReturnType; + interface InspectorOptions { /** * The websocket URL exposed by Workers that the inspector should connect to. @@ -54,16 +133,7 @@ interface InspectorOptions { sourceMapPath?: string | undefined; } -export interface ErrorProperties { - message?: string; - cause?: unknown; - stack?: string; -} - -export function connectToInspector({ - inspectorUrl, - sourceMapPath, -}: InspectorOptions) { +function connectToInspector({inspectorUrl, sourceMapPath}: InspectorOptions) { /** * A simple decrementing id to attach to messages sent to DevTools. * Use negative ids to void collisions with DevTools messages. @@ -272,70 +342,15 @@ export function connectToInspector({ })()); }; - ws.addEventListener('message', async (event: MessageEvent) => { - if (typeof event.data === 'string') { - const evt = JSON.parse(event.data); - cleanupMessageQueue(evt); - - if (evt.method === 'Runtime.exceptionThrown') { - const params = evt.params as Protocol.Runtime.ExceptionThrownEvent; - - const errorProperties: ErrorProperties = {}; - - const sourceMapConsumer = await getSourceMapConsumer(); - if (sourceMapConsumer !== undefined) { - // Create the lines for the exception details log - const message = - params.exceptionDetails.exception?.description?.split('\n')[0]; - const stack = params.exceptionDetails.stackTrace?.callFrames; - const formatted = formatStructuredError( - sourceMapConsumer, - message, - stack, - ); - - errorProperties.message = params.exceptionDetails.text; - errorProperties.stack = formatted; - } else { - errorProperties.message = - params.exceptionDetails.text + - ' ' + - (params.exceptionDetails.exception?.description ?? ''); - } - - console.error( - await reconstructError( - errorProperties, - params.exceptionDetails.exception, - ), - ); - } - - if (evt.method === 'Runtime.consoleAPICalled') { - const params = evt.params as Protocol.Runtime.ConsoleAPICalledEvent; - await logConsoleMessage(params, reconstructError); - } - } else { - // We should never get here, but who know is 2022... - console.error('Unrecognised devtools event:', event); - } - }); - ws.once('open', () => { send('Runtime.enable'); - // TODO: Why does this need a timeout? - // setTimeout(() => send('Network.enable'), 2000); + // Keep the websocket alive by sending a message every 10 seconds keepAliveInterval = setInterval(() => send('Runtime.getIsolateId'), 10_000); }); ws.on('unexpected-response', () => { console.log('Waiting for connection...'); - /** - * This usually means the worker is not "ready" yet - * so we'll just retry the connection process - */ - // retryRemoteWebSocketConnection(); }); ws.once('close', () => { @@ -345,6 +360,11 @@ export function connectToInspector({ return { ws, + send, + reconstructError, + getSourceMapConsumer, + cleanupMessageQueue, + isClosed, close: () => { clearInterval(keepAliveInterval); diff --git a/packages/cli/src/lib/mini-oxygen/workerd.ts b/packages/cli/src/lib/mini-oxygen/workerd.ts index e9a8e87763..4680967035 100644 --- a/packages/cli/src/lib/mini-oxygen/workerd.ts +++ b/packages/cli/src/lib/mini-oxygen/workerd.ts @@ -16,8 +16,7 @@ import { } from '@shopify/cli-kit/node/fs'; import {renderSuccess} from '@shopify/cli-kit/node/ui'; import {lookupMimeType} from '@shopify/cli-kit/node/mimes'; -import {connectToInspector, findInspectorUrl} from './workerd-inspector.js'; -import {createInspectorProxy} from './workerd-inspector-proxy.js'; +import {createInspectorConnector} from './workerd-inspector.js'; import {findPort} from '../find-port.js'; import type {MiniOxygenInstance, MiniOxygenOptions} from './types.js'; import {OXYGEN_HEADERS_MAP, logRequestLine} from './common.js'; @@ -42,7 +41,7 @@ export async function startWorkerdServer({ buildPathClient, env, }: MiniOxygenOptions): Promise { - const workerdInspectorPort = await findPort(PRIVATE_WORKERD_INSPECTOR_PORT); + const privateInspectorPort = await findPort(PRIVATE_WORKERD_INSPECTOR_PORT); const oxygenHeadersMap = Object.values(OXYGEN_HEADERS_MAP).reduce( (acc, item) => { @@ -61,7 +60,7 @@ export async function startWorkerdServer({ cf: false, verbose: false, port: appPort, - inspectorPort: workerdInspectorPort, + inspectorPort: privateInspectorPort, log: new NoOpLog(), liveReload: watch, host: 'localhost', @@ -109,18 +108,15 @@ export async function startWorkerdServer({ const sourceMapPath = buildPathWorkerFile + '.map'; - let inspectorUrl = await findInspectorUrl(workerdInspectorPort); - let inspectorConnection = inspectorUrl - ? connectToInspector({inspectorUrl, sourceMapPath}) - : undefined; + const reconnect = createInspectorConnector({ + debug, + sourceMapPath, + absoluteBundlePath, + privateInspectorPort, + publicInspectorPort, + }); - const inspectorProxy = debug - ? createInspectorProxy( - publicInspectorPort, - absoluteBundlePath, - inspectorConnection, - ) - : undefined; + await reconnect(); return { port: appPort, @@ -138,18 +134,15 @@ export async function startWorkerdServer({ } } - inspectorConnection?.close(); - - // @ts-expect-error - await miniOxygen.setOptions(miniOxygenOptions); - inspectorUrl ??= await findInspectorUrl(workerdInspectorPort); - if (inspectorUrl) { - inspectorConnection = connectToInspector({inspectorUrl, sourceMapPath}); - inspectorProxy?.updateInspectorConnection(inspectorConnection); - } + await reconnect(() => miniOxygen.setOptions(miniOxygenOptions as any)); }, showBanner(options) { - console.log(''); + console.log(''); // New line + + const debuggerMessage = `\n\nDebug mode enabled. Attach a ${ + process.env.TERM_PROGRAM === 'vscode' ? 'VSCode ' : '' + }debugger to port ${publicInspectorPort}\nor open DevTools in http://localhost:${publicInspectorPort}`; + renderSuccess({ headline: `${ options?.headlinePrefix ?? '' @@ -159,13 +152,7 @@ export async function startWorkerdServer({ body: [ `View ${options?.appName ?? 'Hydrogen'} app: ${listeningAt}`, ...(options?.extraLines ?? []), - ...(debug - ? [ - { - warn: `\n\nDebugger listening on ws://localhost:${publicInspectorPort}`, - }, - ] - : []), + ...(debug ? [{warn: debuggerMessage}] : []), ], }); console.log('');