From dd1a75636d3bd35e42c6a8e4a52f64b3346be134 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 9 Nov 2024 11:30:23 +0900 Subject: [PATCH 1/5] fix(ssr): preserve fetchModule error details --- .prettierignore | 1 + packages/vite/src/node/server/hmr.ts | 1 + .../__snapshots__/ssrLoadModule.spec.ts.snap | 57 +++++++++++++++++++ .../fixtures/errors/syntax-error-dep.js | 1 + .../fixtures/errors/syntax-error-dep.ts | 1 + .../__tests__/fixtures/errors/syntax-error.js | 1 + .../__tests__/fixtures/errors/syntax-error.ts | 1 + .../node/ssr/__tests__/ssrLoadModule.spec.ts | 34 +++++++++++ .../vite/src/shared/moduleRunnerTransport.ts | 8 ++- 9 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap create mode 100644 packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.js create mode 100644 packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.ts create mode 100644 packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.js create mode 100644 packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.ts diff --git a/.prettierignore b/.prettierignore index 95541be4525222..d5283c0ed86f17 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,5 +1,6 @@ packages/*/CHANGELOG.md packages/vite/src/node/ssr/runtime/__tests__/fixtures +packages/vite/src/node/ssr/__tests__/fixtures/errors playground-temp/ dist/ temp/ diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index 18c9537d024e0e..60db76c9361958 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -213,6 +213,7 @@ export const normalizeHotChannel = ( name: error.name, message: error.message, stack: error.stack, + ...error, // preserve enumerable properties such as RollupError.loc, frame, plugin }, } } diff --git a/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap b/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap new file mode 100644 index 00000000000000..7b9a13e9fb1762 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap @@ -0,0 +1,57 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`parse error 1`] = ` +{ + "frame": " +Expected ";" but found "code" +1 | invalid code + | ^ +2 | +", + "id": "/fixtures/errors/syntax-error.ts", + "loc": { + "column": 8, + "file": "/fixtures/errors/syntax-error.ts", + "line": 1, + }, + "message": "Transform failed with 1 error: +/fixtures/errors/syntax-error.ts:1:8: ERROR: Expected ";" but found "code"", +} +`; + +exports[`parse error 2`] = ` +{ + "frame": "", + "id": "", + "loc": undefined, + "message": "Expected ';', '}' or ", +} +`; + +exports[`parse error 3`] = ` +{ + "frame": " +Expected ";" but found "code" +1 | invalid code + | ^ +2 | +", + "id": "/fixtures/errors/syntax-error.ts", + "loc": { + "column": 8, + "file": "/fixtures/errors/syntax-error.ts", + "line": 1, + }, + "message": "Transform failed with 1 error: +/fixtures/errors/syntax-error.ts:1:8: ERROR: Expected ";" but found "code"", +} +`; + +exports[`parse error 4`] = ` +{ + "frame": "", + "id": "", + "loc": undefined, + "message": "Expected ';', '}' or ", +} +`; diff --git a/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.js b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.js new file mode 100644 index 00000000000000..defb897f2c0132 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.js @@ -0,0 +1 @@ +import './syntax-error.js' diff --git a/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.ts b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.ts new file mode 100644 index 00000000000000..af356b5c570091 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error-dep.ts @@ -0,0 +1 @@ +import './syntax-error.ts' diff --git a/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.js b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.js new file mode 100644 index 00000000000000..618a16e1c99712 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.js @@ -0,0 +1 @@ +invalid code diff --git a/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.ts b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.ts new file mode 100644 index 00000000000000..618a16e1c99712 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/fixtures/errors/syntax-error.ts @@ -0,0 +1 @@ +invalid code diff --git a/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts index b5bae3ae1745f7..4479d20be233c5 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts @@ -1,5 +1,6 @@ import { fileURLToPath } from 'node:url' import path from 'node:path' +import { stripVTControlCharacters } from 'node:util' import { expect, test } from 'vitest' import { createServer } from '../../server' import { normalizePath } from '../../utils' @@ -178,3 +179,36 @@ test('can access nodejs global', async () => { const mod = await server.ssrLoadModule('/fixtures/global/test.js') expect(mod.default).toBe(globalThis) }) + +test('parse error', async () => { + const server = await createDevServer() + + function stripRoot(s?: string) { + return (s || '').replace(server.config.root, '') + } + + for (const file of [ + '/fixtures/errors/syntax-error.ts', + '/fixtures/errors/syntax-error.js', + '/fixtures/errors/syntax-error-dep.ts', + '/fixtures/errors/syntax-error-dep.js', + ]) { + try { + await server.ssrLoadModule(file) + } catch (e) { + expect(e).toBeInstanceOf(Error) + expect({ + message: stripRoot(e.message), + frame: stripVTControlCharacters(e.frame || ''), + id: stripRoot(e.id), + loc: e.loc && { + file: stripRoot(e.loc.file), + column: e.loc.column, + line: e.loc.line, + }, + }).toMatchSnapshot() + continue + } + expect.unreachable() + } +}) diff --git a/packages/vite/src/shared/moduleRunnerTransport.ts b/packages/vite/src/shared/moduleRunnerTransport.ts index f643e3d6a68c4a..6d5bd9a689108a 100644 --- a/packages/vite/src/shared/moduleRunnerTransport.ts +++ b/packages/vite/src/shared/moduleRunnerTransport.ts @@ -32,6 +32,10 @@ type InvokeableModuleRunnerTransport = Omit & { ): Promise>> } +function reviveInvokeError(e: any) { + return Object.assign(new Error(e.message || 'Unknown invoke error'), e) +} + const createInvokeableTransport = ( transport: ModuleRunnerTransport, ): InvokeableModuleRunnerTransport => { @@ -49,7 +53,7 @@ const createInvokeableTransport = ( } satisfies InvokeSendData, } satisfies CustomPayload) if ('e' in result) { - throw result.e + throw reviveInvokeError(result.e) } return result.r }, @@ -90,7 +94,7 @@ const createInvokeableTransport = ( const { e, r } = data.data if (e) { - promise.reject(e) + promise.reject(reviveInvokeError(e)) } else { promise.resolve(r) } From 2b243fe8da7773c67c87b2bce82c345e4241ffe9 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Fri, 8 Nov 2024 17:33:04 +0900 Subject: [PATCH 2/5] fix(ssr): format ssrTransform parse error --- packages/vite/src/node/ssr/ssrTransform.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index a93b2c9d8a73ba..b5bcd249b4c972 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -19,7 +19,12 @@ import { walk as eswalk } from 'estree-walker' import type { RawSourceMap } from '@ampproject/remapping' import { parseAstAsync as rollupParseAstAsync } from 'rollup/parseAst' import type { TransformResult } from '../server/transformRequest' -import { combineSourcemaps, isDefined } from '../utils' +import { + combineSourcemaps, + generateCodeFrame, + isDefined, + numberToPos, +} from '../utils' import { isJSONRequest } from '../plugins/json' import type { DefineImportMetadata } from '../../shared/ssrTransform' @@ -81,15 +86,12 @@ async function ssrTransformScript( try { ast = await rollupParseAstAsync(code) } catch (err) { - if (!err.loc || !err.loc.line) throw err - const line = err.loc.line - throw new Error( - `Parse failure: ${ - err.message - }\nAt file: ${url}\nContents of line ${line}: ${ - code.split('\n')[line - 1] - }`, - ) + if (err.code === 'PARSE_ERROR' && typeof err.pos === 'number') { + err.loc = numberToPos(code, err.pos) + err.loc.file = url + err.frame = generateCodeFrame(code, err.pos) + } + throw err } let uid = 0 From 433726e6fdb5f2e91108f3a888a461f51abb7d03 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Fri, 8 Nov 2024 17:45:47 +0900 Subject: [PATCH 3/5] test: add test --- .../node/ssr/__tests__/ssrTransform.spec.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index fa8bd6e52a302c..29935b60c79dd1 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -1205,3 +1205,25 @@ console.log(bar) " `) }) + +test('parse error', async () => { + try { + await ssrTransform(`some bad code`, null, '/file.js', '') + expect.unreachable() + } catch (e) { + expect(e).toMatchInlineSnapshot(`[RollupError: Expected ';', '}' or ]`) + expect({ ...e }).toMatchInlineSnapshot(` + { + "code": "PARSE_ERROR", + "frame": "1 | some bad code + | ^", + "loc": { + "column": 6, + "file": "/file.js", + "line": 1, + }, + "pos": 5, + } + `) + } +}) From db31543d10a6b85a7f440b09b15e6f25ad5346fa Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 9 Nov 2024 12:06:37 +0900 Subject: [PATCH 4/5] chore: replace test --- .../__snapshots__/ssrLoadModule.spec.ts.snap | 20 +++++++++++++---- .../node/ssr/__tests__/ssrTransform.spec.ts | 22 ------------------- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap b/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap index 7b9a13e9fb1762..a920a5965c67db 100644 --- a/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap +++ b/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap @@ -21,9 +21,15 @@ Expected ";" but found "code" exports[`parse error 2`] = ` { - "frame": "", + "frame": "1 | invalid code + | ^ +2 | ", "id": "", - "loc": undefined, + "loc": { + "column": 9, + "file": "/fixtures/errors/syntax-error.js", + "line": 1, + }, "message": "Expected ';', '}' or ", } `; @@ -49,9 +55,15 @@ Expected ";" but found "code" exports[`parse error 4`] = ` { - "frame": "", + "frame": "1 | invalid code + | ^ +2 | ", "id": "", - "loc": undefined, + "loc": { + "column": 9, + "file": "/fixtures/errors/syntax-error.js", + "line": 1, + }, "message": "Expected ';', '}' or ", } `; diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index 29935b60c79dd1..fa8bd6e52a302c 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -1205,25 +1205,3 @@ console.log(bar) " `) }) - -test('parse error', async () => { - try { - await ssrTransform(`some bad code`, null, '/file.js', '') - expect.unreachable() - } catch (e) { - expect(e).toMatchInlineSnapshot(`[RollupError: Expected ';', '}' or ]`) - expect({ ...e }).toMatchInlineSnapshot(` - { - "code": "PARSE_ERROR", - "frame": "1 | some bad code - | ^", - "loc": { - "column": 6, - "file": "/file.js", - "line": 1, - }, - "pos": 5, - } - `) - } -}) From 6b0ea325c3d6c27106e397281d735fd524ff679b Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 9 Nov 2024 12:23:39 +0900 Subject: [PATCH 5/5] fix: err.id too --- .../ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap | 4 ++-- packages/vite/src/node/ssr/ssrTransform.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap b/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap index a920a5965c67db..bcb392884b3f6c 100644 --- a/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap +++ b/packages/vite/src/node/ssr/__tests__/__snapshots__/ssrLoadModule.spec.ts.snap @@ -24,7 +24,7 @@ exports[`parse error 2`] = ` "frame": "1 | invalid code | ^ 2 | ", - "id": "", + "id": "/fixtures/errors/syntax-error.js", "loc": { "column": 9, "file": "/fixtures/errors/syntax-error.js", @@ -58,7 +58,7 @@ exports[`parse error 4`] = ` "frame": "1 | invalid code | ^ 2 | ", - "id": "", + "id": "/fixtures/errors/syntax-error.js", "loc": { "column": 9, "file": "/fixtures/errors/syntax-error.js", diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index b5bcd249b4c972..7375a06cdf4a0a 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -87,6 +87,7 @@ async function ssrTransformScript( ast = await rollupParseAstAsync(code) } catch (err) { if (err.code === 'PARSE_ERROR' && typeof err.pos === 'number') { + err.id = url err.loc = numberToPos(code, err.pos) err.loc.file = url err.frame = generateCodeFrame(code, err.pos)