From 7182cee62953ddcd8909b399f757846051966256 Mon Sep 17 00:00:00 2001 From: sanish chirayath Date: Tue, 24 Mar 2026 15:31:21 +0530 Subject: [PATCH] fix: enhance error handling and context retrieval for script errors (#7537) * refactor: enhance error handling and context retrieval for script errors - Updated the error formatter to utilize in-memory script content for error context, improving accuracy when users have unsaved changes. - Introduced a new utility function, `getSourceContextFromContent`, to extract context lines from in-memory scripts. - Enhanced tests to verify that draft script errors display the correct code context, ensuring users see the most relevant information during debugging. - Added new Playwright tests to validate error handling in draft states across pre-request, post-response, and test scripts. * refactor: enhance error context retrieval in error formatter - Updated `getSourceContext` to accept in-memory content, improving context extraction for unsaved changes. - Deprecated `getSourceContextFromContent` in favor of the new parameterized approach. - Adjusted related tests to ensure accurate context handling for draft script errors. * refactor: enhance error context handling for draft scripts * refactor: streamline script error tests and enhance utility functions - Consolidated helper functions for sending requests and waiting for responses into the actions module. - Introduced new utility functions for selecting script sub-tabs and editing CodeMirror editors. - Updated test cases to utilize the new utility functions, improving readability and maintainability. - Enhanced locators for better integration with testing frameworks. * refactor: improve script error context handling and utility functions - Introduced a new utility function to streamline the retrieval of script block start lines for .bru and .yml files. - Enhanced the error formatter to prioritize in-memory draft content when resolving error contexts, improving accuracy for unsaved changes. - Consolidated context extraction logic into a single function to reduce redundancy and improve maintainability. - Updated related tests to ensure accurate context handling for both draft and disk-based scripts. * refactor: add comments to clarify line index calculations in error formatter --- .../src/components/RequestPane/Tests/index.js | 26 +- .../bruno-electron/src/utils/collection.js | 57 ++-- .../tests/utils/collection.spec.js | 71 ++++- .../bruno-js/src/utils/error-formatter.js | 152 +++++++++-- .../src/utils/error-formatter.spec.js | 250 +++++++++++++++++- .../script-errors/draft-script-errors.spec.ts | 120 +++++++++ .../script-errors-test/draft-error-test.bru | 16 ++ .../script-errors-test/draft-postres-test.bru | 16 ++ .../script-errors-test/draft-tests-test.bru | 16 ++ tests/script-errors/script-errors.spec.ts | 22 +- tests/utils/page/actions.ts | 68 ++++- tests/utils/page/locators.ts | 6 +- 12 files changed, 735 insertions(+), 85 deletions(-) create mode 100644 tests/script-errors/draft-script-errors.spec.ts create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/draft-error-test.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/draft-postres-test.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/draft-tests-test.bru diff --git a/packages/bruno-app/src/components/RequestPane/Tests/index.js b/packages/bruno-app/src/components/RequestPane/Tests/index.js index b9c9633be..534c15145 100644 --- a/packages/bruno-app/src/components/RequestPane/Tests/index.js +++ b/packages/bruno-app/src/components/RequestPane/Tests/index.js @@ -27,18 +27,20 @@ const Tests = ({ item, collection }) => { const onSave = () => dispatch(saveRequest(item.uid, collection.uid)); return ( - +
+ +
); }; diff --git a/packages/bruno-electron/src/utils/collection.js b/packages/bruno-electron/src/utils/collection.js index 5b7017929..5ea2d49af 100644 --- a/packages/bruno-electron/src/utils/collection.js +++ b/packages/bruno-electron/src/utils/collection.js @@ -253,6 +253,9 @@ const mergeScripts = (collection, request, requestTreePath, scriptFlow) => { displayPath: config.collectionFile }; + const withContent = (source, script) => + script?.trim() ? { ...source, scriptContent: script } : source; + let combinedPreReqScript = []; let combinedPreReqSources = []; let combinedPostResScript = []; @@ -271,81 +274,99 @@ const mergeScripts = (collection, request, requestTreePath, scriptFlow) => { let preReqScript = get(folderRoot, 'request.script.req', ''); if (preReqScript && preReqScript.trim() !== '') { combinedPreReqScript.push(preReqScript); - combinedPreReqSources.push(folderSource); + combinedPreReqSources.push(withContent(folderSource, preReqScript)); } let postResScript = get(folderRoot, 'request.script.res', ''); if (postResScript && postResScript.trim() !== '') { combinedPostResScript.push(postResScript); - combinedPostResSources.push(folderSource); + combinedPostResSources.push(withContent(folderSource, postResScript)); } let tests = get(folderRoot, 'request.tests', ''); if (tests && tests?.trim?.() !== '') { combinedTests.push(tests); - combinedTestsSources.push(folderSource); + combinedTestsSources.push(withContent(folderSource, tests)); } } } + // Capture original request script content before overwriting with combined code + const originalPreReqScript = request?.script?.req || ''; + const originalPostResScript = request?.script?.res || ''; + const originalTests = request?.tests || ''; + + // Wrap scripts, join them, and annotate metadata with the original request script content. + // Returns { code, metadata } where metadata.requestScriptContent is set. + const buildCombinedScript = (scripts, requestIndex, sources, originalScript) => { + const result = wrapAndJoinScripts(scripts, requestIndex, sources); + if (result.metadata) { + result.metadata.requestScriptContent = originalScript; + } + return result; + }; + // Wrap each script segment in its own closure and join them // This allows each script to run separately with its own scope, // preventing variable re-declaration errors and allowing early returns // to only affect that specific script segment + const collectionPreReqSource = withContent(collectionSource, collectionPreReqScript); const preReqScripts = [ collectionPreReqScript, ...combinedPreReqScript, - request?.script?.req || '' + originalPreReqScript ]; - const preReqSources = [collectionSource, ...combinedPreReqSources, null]; - const preReq = wrapAndJoinScripts(preReqScripts, preReqScripts.length - 1, preReqSources); + const preReqSources = [collectionPreReqSource, ...combinedPreReqSources, null]; + const preReq = buildCombinedScript(preReqScripts, preReqScripts.length - 1, preReqSources, originalPreReqScript); request.script.req = preReq.code; request.script.reqMetadata = preReq.metadata; // Handle post-response scripts based on scriptFlow + const collectionPostResSource = withContent(collectionSource, collectionPostResScript); if (scriptFlow === 'sequential') { const postResScripts = [ collectionPostResScript, ...combinedPostResScript, - request?.script?.res || '' + originalPostResScript ]; - const postResSources = [collectionSource, ...combinedPostResSources, null]; - const postRes = wrapAndJoinScripts(postResScripts, postResScripts.length - 1, postResSources); + const postResSources = [collectionPostResSource, ...combinedPostResSources, null]; + const postRes = buildCombinedScript(postResScripts, postResScripts.length - 1, postResSources, originalPostResScript); request.script.res = postRes.code; request.script.resMetadata = postRes.metadata; } else { // Reverse order for non-sequential flow const postResScripts = [ - request?.script?.res || '', + originalPostResScript, ...[...combinedPostResScript].reverse(), collectionPostResScript ]; - const postResSources = [null, ...[...combinedPostResSources].reverse(), collectionSource]; - const postRes = wrapAndJoinScripts(postResScripts, 0, postResSources); + const postResSources = [null, ...[...combinedPostResSources].reverse(), collectionPostResSource]; + const postRes = buildCombinedScript(postResScripts, 0, postResSources, originalPostResScript); request.script.res = postRes.code; request.script.resMetadata = postRes.metadata; } // Handle tests based on scriptFlow + const collectionTestsSource = withContent(collectionSource, collectionTests); if (scriptFlow === 'sequential') { const testScripts = [ collectionTests, ...combinedTests, - request?.tests || '' + originalTests ]; - const testSources = [collectionSource, ...combinedTestsSources, null]; - const tests = wrapAndJoinScripts(testScripts, testScripts.length - 1, testSources); + const testSources = [collectionTestsSource, ...combinedTestsSources, null]; + const tests = buildCombinedScript(testScripts, testScripts.length - 1, testSources, originalTests); request.tests = tests.code; request.testsMetadata = tests.metadata; } else { // Reverse order for non-sequential flow const testScripts = [ - request?.tests || '', + originalTests, ...[...combinedTests].reverse(), collectionTests ]; - const testSources = [null, ...[...combinedTestsSources].reverse(), collectionSource]; - const tests = wrapAndJoinScripts(testScripts, 0, testSources); + const testSources = [null, ...[...combinedTestsSources].reverse(), collectionTestsSource]; + const tests = buildCombinedScript(testScripts, 0, testSources, originalTests); request.tests = tests.code; request.testsMetadata = tests.metadata; } diff --git a/packages/bruno-electron/tests/utils/collection.spec.js b/packages/bruno-electron/tests/utils/collection.spec.js index 8aec614a5..4010eade7 100644 --- a/packages/bruno-electron/tests/utils/collection.spec.js +++ b/packages/bruno-electron/tests/utils/collection.spec.js @@ -408,7 +408,8 @@ describe('mergeScripts metadata', () => { mergeScripts(collection, request, [request], 'sequential'); expect(request.script.reqMetadata).toEqual({ requestStartLine: 1, - requestEndLine: 3 + requestEndLine: 3, + requestScriptContent: 'console.log("req");' }); }); @@ -474,4 +475,72 @@ describe('mergeScripts metadata', () => { expect(request.script.reqMetadata.segments[0].displayPath).toBe('collection.bru'); }); + + test('includes requestScriptContent in metadata for pre-request scripts', () => { + const collection = makeCollection({ preReq: 'let col = 1;' }); + const request = makeRequest({ preReq: 'let req = 2;' }); + mergeScripts(collection, request, [request], 'sequential'); + + expect(request.script.reqMetadata.requestScriptContent).toBe('let req = 2;'); + }); + + test('includes requestScriptContent in metadata for post-response scripts', () => { + const collection = makeCollection({ postRes: 'let col = 1;' }); + const request = makeRequest({ postRes: 'let req = 2;' }); + mergeScripts(collection, request, [request], 'sequential'); + + expect(request.script.resMetadata.requestScriptContent).toBe('let req = 2;'); + }); + + test('includes requestScriptContent in metadata for tests', () => { + const collection = makeCollection({ tests: 'test("col", () => {});' }); + const request = makeRequest({ tests: 'test("req", () => {});' }); + mergeScripts(collection, request, [request], 'sequential'); + + expect(request.testsMetadata.requestScriptContent).toBe('test("req", () => {});'); + }); + + test('includes requestScriptContent as empty string when request script is empty', () => { + const collection = makeCollection({ preReq: 'let col = 1;' }); + const request = makeRequest(); + mergeScripts(collection, request, [request], 'sequential'); + + expect(request.script.reqMetadata.requestScriptContent).toBe(''); + }); + + test('includes scriptContent in collection segment sources', () => { + const collection = makeCollection({ preReq: 'let col = 1;' }); + const request = makeRequest({ preReq: 'let req = 2;' }); + mergeScripts(collection, request, [request], 'sequential'); + + expect(request.script.reqMetadata.segments[0].scriptContent).toBe('let col = 1;'); + }); + + test('includes scriptContent in folder segment sources', () => { + const collection = makeCollection(); + const folder = makeFolder('subfolder', { preReq: 'let fold = 1;' }); + const request = makeRequest({ preReq: 'let req = 2;' }); + mergeScripts(collection, request, [folder, request], 'sequential'); + + expect(request.script.reqMetadata.segments[0].scriptContent).toBe('let fold = 1;'); + }); + + test('includes scriptContent for both collection and folder segments', () => { + const collection = makeCollection({ preReq: 'let col = 1;' }); + const folder = makeFolder('subfolder', { preReq: 'let fold = 2;' }); + const request = makeRequest({ preReq: 'let req = 3;' }); + mergeScripts(collection, request, [folder, request], 'sequential'); + + expect(request.script.reqMetadata.segments).toHaveLength(2); + expect(request.script.reqMetadata.segments[0].scriptContent).toBe('let col = 1;'); + expect(request.script.reqMetadata.segments[1].scriptContent).toBe('let fold = 2;'); + }); + + test('includes requestScriptContent in non-sequential post-response metadata', () => { + const collection = makeCollection({ postRes: 'let col = 1;' }); + const request = makeRequest({ postRes: 'let req = 2;' }); + mergeScripts(collection, request, [request], 'non-sequential'); + + expect(request.script.resMetadata.requestScriptContent).toBe('let req = 2;'); + }); }); diff --git a/packages/bruno-js/src/utils/error-formatter.js b/packages/bruno-js/src/utils/error-formatter.js index 00f8d2f2f..ca8e56c00 100644 --- a/packages/bruno-js/src/utils/error-formatter.js +++ b/packages/bruno-js/src/utils/error-formatter.js @@ -241,6 +241,13 @@ const adjustLineNumber = (filePath, reportedLine, isQuickJS, scriptType = null, return scriptRelativeLine; }; +/** Look up the script block start line for a .bru or .yml file */ +const findBlockStart = (filePath, scriptType, cache) => { + if (filePath.endsWith('.bru')) return findScriptBlockStartLine(filePath, scriptType, cache); + if (filePath.endsWith('.yml')) return findYmlScriptBlockStartLine(filePath, scriptType, cache); + return null; +}; + /** * Resolve an error in a collection/folder script segment to its source file and line. * Uses the segments array in metadata to find which segment the error falls in, @@ -255,19 +262,34 @@ const resolveSegmentError = (parsed, metadata, scriptType, cache) => { for (const segment of metadata.segments) { if (scriptRelativeLine >= segment.startLine && scriptRelativeLine <= segment.endLine) { - const isBru = segment.filePath.endsWith('.bru'); - const isYml = segment.filePath.endsWith('.yml'); - if (!isBru && !isYml) return null; + if (!isAllowedSourceFile(segment.filePath)) return null; - const blockStartLine = isBru - ? findScriptBlockStartLine(segment.filePath, scriptType, cache) - : findYmlScriptBlockStartLine(segment.filePath, scriptType, cache); - if (!blockStartLine) return null; + const blockStartLine = findBlockStart(segment.filePath, scriptType, cache); + if (!blockStartLine) { + // No script block on disk — only possible when user added a new script as a draft. + // If we have in-memory content, return it so the caller can show the code snippet. + if (segment.scriptContent) { + return { + line: null, + filePath: segment.filePath, + displayPath: segment.displayPath, + scriptContent: segment.scriptContent, + // segment.startLine points to the IIFE wrapper line (`await (async () => {`), + // so subtracting it yields a 1-based index into the user's script content. + lineInScript: scriptRelativeLine - segment.startLine + }; + } + return null; + } return { line: blockStartLine + (scriptRelativeLine - segment.startLine) - 1, filePath: segment.filePath, - displayPath: segment.displayPath + displayPath: segment.displayPath, + scriptContent: segment.scriptContent || null, + // segment.startLine points to the IIFE wrapper line (`await (async () => {`), + // so subtracting it yields a 1-based index into the user's script content. + lineInScript: scriptRelativeLine - segment.startLine }; } } @@ -332,12 +354,10 @@ const parseErrorLocation = (error) => { return parsed; }; -/** Read source file and extract context lines around the error location */ -const getSourceContext = (filePath, errorLine, contextLines = DEFAULT_CONTEXT_LINES, cache = null) => { - const content = readFile(filePath, cache); - if (!content) return null; +/** Build a context-lines object from an array of source lines around an error */ +const buildContextLines = (lines, errorLine, contextLines) => { + if (errorLine < 1 || errorLine > lines.length) return null; - const lines = content.split('\n'); const startLine = Math.max(1, errorLine - contextLines); const endLine = Math.min(lines.length, errorLine + contextLines); @@ -353,6 +373,19 @@ const getSourceContext = (filePath, errorLine, contextLines = DEFAULT_CONTEXT_LI return { lines: contextLinesArray, startLine, errorLine }; }; +/** Read source file and extract context lines around the error location */ +const getSourceContext = (filePath, errorLine, contextLines = DEFAULT_CONTEXT_LINES, cache = null) => { + const content = readFile(filePath, cache); + if (!content) return null; + return buildContextLines(content.split('\n'), errorLine, contextLines); +}; + +/** Extract context lines from in-memory script content (e.g. unsaved draft scripts) */ +const getSourceContextFromContent = (content, errorLine, contextLines = DEFAULT_CONTEXT_LINES) => { + if (!content) return null; + return buildContextLines(content.split('\n'), errorLine, contextLines); +}; + /** Build adjusted stack trace string from structured CallSite data */ const buildStackFromCallSites = (callSites, scriptType = null, cache = null, scriptMetadata = null) => { return callSites.map((site) => { @@ -364,7 +397,7 @@ const buildStackFromCallSites = (callSites, scriptType = null, cache = null, scr if (adjusted === null && scriptMetadata?.segments) { const parsed = { line: site.line, isQuickJS: false }; const resolved = resolveSegmentError(parsed, scriptMetadata, scriptType, cache); - if (resolved) { + if (resolved && resolved.line !== null) { fileToUse = resolved.filePath; lineToUse = resolved.line; } @@ -391,7 +424,7 @@ const adjustStackTrace = (stack, scriptType = null, cache = null, scriptMetadata if (adjusted === null && scriptMetadata?.segments) { const parsed = { line: match.line, isQuickJS }; const resolved = resolveSegmentError(parsed, scriptMetadata, scriptType, cache); - if (resolved) { + if (resolved && resolved.line !== null) { const suffix = match.isQuickJS ? ')' : ''; return match.column !== null ? line.replace(`${match.filePath}:${match.line}:${match.column}${suffix}`, `${resolved.filePath}:${resolved.line}:${match.column}${suffix}`) @@ -547,6 +580,48 @@ const formatErrorWithContext = (error, relativeFilePath = null, scriptType = nul * @param {string} collectionPath - Absolute path to the collection root (used to compute relative display paths) * @returns {object|null} Structured error context or null */ +/** + * Resolve error context, preferring in-memory draft content over disk. + * + * Three resolution paths (tried in order): + * 1. Request-level error with in-memory draft content + * 2. Segment (collection/folder) error with in-memory draft content + * 3. Disk-based file read (original behavior) + * + * @returns {{ context, fromMemory, draftOnlyBlock }|null} + */ +const resolveErrorContext = ({ adjustedLine, scriptRelativeLine, metadata, segmentResult, filePath, sourceFile, sourceLine, scriptType, cache }) => { + // Request-level error with in-memory draft content + if (adjustedLine !== null && metadata?.requestScriptContent) { + // Check whether the script block exists on disk. When the user added a brand-new + // script that hasn't been saved yet, findBlockStart returns null and adjustLineNumber + // returned scriptRelativeLine (not a real .bru file line), so stack frame adjustment + // would produce misleading results — flag it as draft-only. + const blockStartLine = findBlockStart(filePath, scriptType, cache); + const draftOnlyBlock = !blockStartLine && isAllowedSourceFile(filePath); + // requestStartLine points to the IIFE wrapper line (`await (async () => {`), + // so subtracting it yields a 1-based index into the user's script content. + const lineInScript = scriptRelativeLine - metadata.requestStartLine; + const context = getSourceContextFromContent(metadata.requestScriptContent, lineInScript, 3); + if (context) return { context, fromMemory: true, draftOnlyBlock }; + } + + // Segment (collection/folder) error with in-memory draft content + if (adjustedLine === null && segmentResult?.scriptContent) { + const context = getSourceContextFromContent(segmentResult.scriptContent, segmentResult.lineInScript, 3); + // segmentResult.line is null when the block doesn't exist on disk + if (context) return { context, fromMemory: true, draftOnlyBlock: segmentResult.line === null }; + } + + // Fall back to reading from disk + if (sourceLine !== null) { + const context = getSourceContext(sourceFile, sourceLine, 3, cache); + if (context) return { context, fromMemory: false, draftOnlyBlock: false }; + } + + return null; +}; + const formatErrorWithContextV2 = (error, scriptType, scriptMetadata, collectionPath) => { if (!error) return null; @@ -559,47 +634,65 @@ const formatErrorWithContextV2 = (error, scriptType, scriptMetadata, collectionP if (!parsed) return null; const { filePath } = parsed; + const wrapperOffset = parsed.isQuickJS ? QUICKJS_SCRIPT_WRAPPER_OFFSET : NODEVM_SCRIPT_WRAPPER_OFFSET; + const scriptRelativeLine = parsed.line - wrapperOffset; const adjustedLine = adjustLineNumber(filePath, parsed.line, parsed.isQuickJS, scriptType, cache, metadata); let sourceFile = filePath; let sourceLine = adjustedLine; // Handle collection/folder script segments + let segmentResult = null; if (adjustedLine === null) { - const segmentResult = resolveSegmentError(parsed, metadata, scriptType, cache); + segmentResult = resolveSegmentError(parsed, metadata, scriptType, cache); if (!segmentResult) return null; sourceFile = segmentResult.filePath; sourceLine = segmentResult.line; } + // Resolve context: prefer in-memory draft content, fall back to disk + const resolved = resolveErrorContext({ + adjustedLine, scriptRelativeLine, metadata, segmentResult, + filePath, sourceFile, sourceLine, scriptType, cache + }); + if (!resolved || resolved.context.lines.length === 0) return null; + + const { context, fromMemory, draftOnlyBlock } = resolved; + const resolvedDisplayPath = posixifyPath( collectionPath ? path.relative(collectionPath, sourceFile) : sourceFile ); - const context = getSourceContext(sourceFile, sourceLine, 3, cache); - if (!context) return null; - const errorType = getErrorTypeName(error); let stack = null; if (error.stack) { - stack = adjustStackTrace(error.stack, scriptType, cache, metadata, parsed.isQuickJS); - // Extract only the stack frames (skip the first line which is the error message) - const stackLines = stack.split('\n').slice(1).filter((l) => l.trim().startsWith('at')); + // When the script block only exists as a draft (not on disk), adjustLineNumber + // cannot map to real .bru file lines — skip adjustment to preserve original frames. + const rawStack = draftOnlyBlock + ? error.stack + : adjustStackTrace(error.stack, scriptType, cache, metadata, parsed.isQuickJS); + const stackLines = rawStack.split('\n').slice(1).filter((l) => l.trim().startsWith('at')); stack = stackLines.length ? stackLines.map((l) => ` ${l.trim()}`).join('\n') : null; } + // When context came from in-memory content, lines are already block-relative + if (fromMemory) { + return { + errorType, + filePath: resolvedDisplayPath, + errorLine: context.errorLine, + lines: context.lines, + stack + }; + } + // Compute block-relative line numbers for the desktop UI. // Users edit scripts in a CodeMirror editor starting at line 1, // so show lines relative to the script block, not absolute .bru file lines. + const blockStartLine = findBlockStart(sourceFile, scriptType, cache); + const isBru = sourceFile.endsWith('.bru'); const isYml = sourceFile.endsWith('.yml'); - - const blockStartLine = isBru - ? findScriptBlockStartLine(sourceFile, scriptType, cache) - : isYml - ? findYmlScriptBlockStartLine(sourceFile, scriptType, cache) - : null; - const blockEndLine = isBru ? findScriptBlockEndLine(sourceFile, scriptType, cache) : isYml @@ -644,6 +737,7 @@ module.exports = { parseErrorLocation, buildStackFromCallSites, getSourceContext, + getSourceContextFromContent, formatErrorWithContext, formatErrorWithContextV2, adjustLineNumber, diff --git a/packages/bruno-js/src/utils/error-formatter.spec.js b/packages/bruno-js/src/utils/error-formatter.spec.js index b77af4475..353ade321 100644 --- a/packages/bruno-js/src/utils/error-formatter.spec.js +++ b/packages/bruno-js/src/utils/error-formatter.spec.js @@ -8,7 +8,10 @@ const { findYmlScriptBlockEndLine, adjustLineNumber, parseStackTrace, - parseErrorLocation + parseErrorLocation, + getSourceContextFromContent, + adjustStackTrace, + buildStackFromCallSites } = require('./error-formatter'); const fs = require('fs'); const path = require('path'); @@ -818,5 +821,250 @@ get { expect(consoleSpy).toHaveBeenCalled(); }); }); + + describe('in-memory script content (draft state)', () => { + it('should use requestScriptContent for request-level errors instead of disk', () => { + // The .bru file on disk has different content than the draft + const draftContent = 'const draft = true;\ndraft.missing.prop;\nconsole.log("draft");'; + const metadata = { + requestStartLine: 1, + requestEndLine: 3, + requestScriptContent: draftContent + }; + + // NodeVM: line 4 - offset 2 = scriptRelativeLine 2, within request range [1,3] + // lineInScript = 2 - 1 = 1, which maps to first line of draft content + const error = makeCallSiteError(bruFilePath, 4, 'draft.missing is not defined', 'ReferenceError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + expect(result.errorLine).toBe(1); + expect(result.lines[0].content).toBe('const draft = true;'); + expect(result.lines[0].isError).toBe(true); + }); + + it('should show correct error line from draft content', () => { + const draftContent = 'const a = 1;\nconst b = undefined;\nb.foo();'; + const metadata = { + requestStartLine: 1, + requestEndLine: 3, + requestScriptContent: draftContent + }; + + // NodeVM: line 5 - offset 2 = scriptRelativeLine 3, within request range [1,3] + // lineInScript = 3 - 1 = 2, which maps to second line of draft content + const error = makeCallSiteError(bruFilePath, 5, 'b is undefined', 'TypeError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + expect(result.errorLine).toBe(2); + const errorLine = result.lines.find((l) => l.isError); + expect(errorLine.content).toBe('const b = undefined;'); + }); + + it('should use scriptContent from segments for collection/folder errors', () => { + const draftCollectionScript = 'const draftCol = true;\ndraftCol.missing.prop;'; + const metadata = { + requestStartLine: 0, + requestEndLine: 0, + segments: [{ + startLine: 1, + endLine: 4, + filePath: collectionYmlPath, + displayPath: 'opencollection.yml', + scriptContent: draftCollectionScript + }] + }; + + // NodeVM: line 4 - offset 2 = scriptRelativeLine 2, falls in segment [1,4] + // lineInScript = 2 - 1 = 1, maps to first line of draft collection script + const error = makeCallSiteError(bruFilePath, 4, 'draftCol.missing is not defined', 'ReferenceError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + expect(result.filePath).toBe('opencollection.yml'); + expect(result.errorLine).toBe(1); + expect(result.lines[0].content).toBe('const draftCol = true;'); + expect(result.lines[0].isError).toBe(true); + }); + + it('should fall back to disk when requestScriptContent is not provided', () => { + const metadata = { + requestStartLine: 1, + requestEndLine: 3 + // no requestScriptContent + }; + + const error = makeCallSiteError(bruFilePath, 3, 'token is not defined', 'ReferenceError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + // Should still work via disk-based fallback + expect(result.filePath).toBe('test.bru'); + }); + + it('should fall back to disk when segment scriptContent is not provided', () => { + const metadata = { + requestStartLine: 0, + requestEndLine: 0, + segments: [{ + startLine: 1, + endLine: 4, + filePath: collectionYmlPath, + displayPath: 'opencollection.yml' + // no scriptContent + }] + }; + + const error = makeCallSiteError(bruFilePath, 4, 'error', 'ReferenceError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + expect(result.filePath).toBe('opencollection.yml'); + }); + + it('should use scriptContent from segments when folder .bru has no script block on disk', () => { + // Create a folder.bru with NO script block (simulates a draft where the user added a new script) + const folderBruPath = path.join(testDir, 'folder.bru'); + fs.writeFileSync(folderBruPath, 'meta {\n name: My Folder\n}\n'); + + const draftFolderScript = 'const x = undefined;\nx.boom();'; + const metadata = { + requestStartLine: 0, + requestEndLine: 0, + segments: [{ + startLine: 1, + endLine: 4, + filePath: folderBruPath, + displayPath: 'folder/folder.bru', + scriptContent: draftFolderScript + }] + }; + + // NodeVM: line 4 - offset 2 = scriptRelativeLine 2, falls in segment [1,4] + // lineInScript = 2 - 1 = 1 + const error = makeCallSiteError(bruFilePath, 4, 'x is undefined', 'TypeError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + expect(result.filePath).toBe('folder.bru'); + expect(result.errorLine).toBe(1); + expect(result.lines[0].content).toBe('const x = undefined;'); + expect(result.lines[0].isError).toBe(true); + }); + + it('should use QuickJS offset correctly with requestScriptContent', () => { + const draftContent = 'const x = 1;\nundefined.boom();'; + const metadata = { + requestStartLine: 1, + requestEndLine: 3, + requestScriptContent: draftContent + }; + + // QuickJS: line 11 - offset 9 = scriptRelativeLine 2, within request range [1,3] + // lineInScript = 2 - 1 = 1 + const error = makeQuickJSError(bruFilePath, 11, 'not defined', 'ReferenceError'); + const result = formatErrorWithContextV2(error, 'pre-request', metadata, testDir); + + expect(result).not.toBeNull(); + expect(result.errorLine).toBe(1); + expect(result.lines[0].content).toBe('const x = 1;'); + }); + }); + }); + + describe('stack trace with null-line segment resolution', () => { + it('buildStackFromCallSites should not interpolate null into stack frames', () => { + // Segment with no on-disk script block → resolveSegmentError returns line: null + const folderBruPath = path.join(testDir, 'folder.bru'); + fs.writeFileSync(folderBruPath, 'meta {\n name: My Folder\n}\n'); + + const metadata = { + requestStartLine: 0, + requestEndLine: 0, + segments: [{ + startLine: 1, + endLine: 4, + filePath: folderBruPath, + displayPath: 'folder/folder.bru', + scriptContent: 'const x = 1;\nx.boom();' + }] + }; + + // NodeVM callSite: line 4 - offset 2 = scriptRelativeLine 2, falls in segment [1,4] + const callSites = [{ filePath: bruFilePath, line: 4, column: 5, functionName: null }]; + const result = buildStackFromCallSites(callSites, 'pre-request', null, metadata); + + expect(result).not.toContain('null'); + // Should fall back to original file/line since resolved.line is null + expect(result).toContain(bruFilePath); + }); + + it('adjustStackTrace should not interpolate null into stack frames', () => { + const folderBruPath = path.join(testDir, 'folder.bru'); + fs.writeFileSync(folderBruPath, 'meta {\n name: My Folder\n}\n'); + + const metadata = { + requestStartLine: 0, + requestEndLine: 0, + segments: [{ + startLine: 1, + endLine: 4, + filePath: folderBruPath, + displayPath: 'folder/folder.bru', + scriptContent: 'const x = 1;\nx.boom();' + }] + }; + + const stack = `TypeError: x.boom is not a function\n at ${bruFilePath}:4:5`; + const result = adjustStackTrace(stack, 'pre-request', null, metadata, false); + + expect(result).not.toContain('null'); + // Original frame should be preserved since resolved.line is null + expect(result).toContain(`${bruFilePath}:4:5`); + }); + }); + + describe('getSourceContextFromContent', () => { + it('should return context lines around the error line', () => { + const content = 'line1\nline2\nline3\nline4\nline5'; + const result = getSourceContextFromContent(content, 3, 1); + + expect(result).not.toBeNull(); + expect(result.errorLine).toBe(3); + expect(result.lines).toHaveLength(3); + expect(result.lines[0]).toEqual({ lineNumber: 2, content: 'line2', isError: false }); + expect(result.lines[1]).toEqual({ lineNumber: 3, content: 'line3', isError: true }); + expect(result.lines[2]).toEqual({ lineNumber: 4, content: 'line4', isError: false }); + }); + + it('should return null for null/empty content', () => { + expect(getSourceContextFromContent(null, 1)).toBeNull(); + expect(getSourceContextFromContent('', 1)).toBeNull(); + }); + + it('should return null for out-of-bounds error line', () => { + const content = 'line1\nline2'; + expect(getSourceContextFromContent(content, 0)).toBeNull(); + expect(getSourceContextFromContent(content, 3)).toBeNull(); + }); + + it('should handle single-line content', () => { + const content = 'only line'; + const result = getSourceContextFromContent(content, 1, 3); + + expect(result).not.toBeNull(); + expect(result.lines).toHaveLength(1); + expect(result.lines[0]).toEqual({ lineNumber: 1, content: 'only line', isError: true }); + }); + + it('should clamp context at file boundaries', () => { + const content = 'line1\nline2\nline3'; + const result = getSourceContextFromContent(content, 1, 5); + + expect(result).not.toBeNull(); + expect(result.lines).toHaveLength(3); + expect(result.startLine).toBe(1); + }); }); }); diff --git a/tests/script-errors/draft-script-errors.spec.ts b/tests/script-errors/draft-script-errors.spec.ts new file mode 100644 index 000000000..11a6e22bf --- /dev/null +++ b/tests/script-errors/draft-script-errors.spec.ts @@ -0,0 +1,120 @@ +import { test, expect } from '../../playwright'; +import { buildScriptErrorLocators, buildCommonLocators } from '../utils/page/locators'; +import { + openRequest, + selectRequestPaneTab, + selectScriptSubTab, + editCodeMirrorEditor, + sendAndWaitForErrorCard, + sendAndWaitForResponse +} from '../utils/page/actions'; +import { setSandboxMode } from '../utils/page/runner'; + +for (const mode of ['safe', 'developer'] as const) { + test.describe.serial(`Draft Script Error Context [${mode} mode]`, () => { + let scriptErrorLocators: ReturnType; + let commonLocators: ReturnType; + + test.beforeAll(async ({ pageWithUserData: page }) => { + scriptErrorLocators = buildScriptErrorLocators(page); + commonLocators = buildCommonLocators(page); + + await setSandboxMode(page, 'script-errors-test', mode); + }); + + test('1. Draft pre-request error shows draft code in error context', async ({ pageWithUserData: page }) => { + await test.step('Open draft-error-test request', async () => { + await openRequest(page, 'script-errors-test', 'draft-error-test'); + }); + + await test.step('Navigate to Script > Pre Request tab and edit script', async () => { + await selectScriptSubTab(page, 'pre-request'); + + await editCodeMirrorEditor( + page, + 'pre-request-script-editor', + 'const draftOnlyVar = "draft";\ndraftOnlyUndefined();' + ); + }); + + await test.step('Verify draft indicator is visible', async () => { + await expect(commonLocators.tabs.draftIndicator()).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Send request and wait for error card', async () => { + await sendAndWaitForErrorCard(page); + }); + + await test.step('Verify error card shows draft code, not saved code', async () => { + const card = scriptErrorLocators.card(); + await expect(scriptErrorLocators.title(card)).toContainText('Pre-Request Script Error'); + await expect(scriptErrorLocators.errorLine(card)).toContainText('draftOnlyUndefined'); + await expect(scriptErrorLocators.errorLine(card)).not.toContainText('savedVar'); + }); + }); + + test('2. Draft post-response error shows draft code in error context', async ({ pageWithUserData: page }) => { + await test.step('Open draft-postres-test request', async () => { + await openRequest(page, 'script-errors-test', 'draft-postres-test'); + }); + + await test.step('Navigate to Script > Post Response tab and edit script', async () => { + await selectScriptSubTab(page, 'post-response'); + + await editCodeMirrorEditor( + page, + 'post-response-script-editor', + 'const postDraftVar = "post-draft";\npostDraftUndefined();' + ); + }); + + await test.step('Verify draft indicator is visible', async () => { + await expect(commonLocators.tabs.draftIndicator()).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Send request and wait for error card', async () => { + await sendAndWaitForResponse(page); + }); + + await test.step('Verify error card shows draft code, not saved code', async () => { + const card = scriptErrorLocators.card(); + await expect(card).toBeVisible(); + await expect(scriptErrorLocators.title(card)).toContainText('Post-Response Script Error'); + await expect(scriptErrorLocators.errorLine(card)).toContainText('postDraftUndefined'); + await expect(scriptErrorLocators.errorLine(card)).not.toContainText('savedData'); + }); + }); + + test('3. Draft test script error shows draft code in error context', async ({ pageWithUserData: page }) => { + await test.step('Open draft-tests-test request', async () => { + await openRequest(page, 'script-errors-test', 'draft-tests-test'); + }); + + await test.step('Navigate to Tests tab and edit script', async () => { + await selectRequestPaneTab(page, 'Tests'); + + await editCodeMirrorEditor( + page, + 'test-script-editor', + 'const draftTest = "test";\ndraftTestUndefined();' + ); + }); + + await test.step('Verify draft indicator is visible', async () => { + await expect(commonLocators.tabs.draftIndicator()).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Send request and wait for response', async () => { + await sendAndWaitForResponse(page); + }); + + await test.step('Verify error card shows draft code, not saved code', async () => { + const card = scriptErrorLocators.card(); + await expect(card).toBeVisible(); + await expect(scriptErrorLocators.title(card)).toContainText('Test Script Error'); + await expect(scriptErrorLocators.errorLine(card)).toContainText('draftTestUndefined'); + await expect(scriptErrorLocators.errorLine(card)).not.toContainText('savedTest'); + }); + }); + }); +} diff --git a/tests/script-errors/fixtures/collections/script-errors-test/draft-error-test.bru b/tests/script-errors/fixtures/collections/script-errors-test/draft-error-test.bru new file mode 100644 index 000000000..7140b580f --- /dev/null +++ b/tests/script-errors/fixtures/collections/script-errors-test/draft-error-test.bru @@ -0,0 +1,16 @@ +meta { + name: draft-error-test + type: http + seq: 9 +} + +get { + url: http://localhost:8081/ping + body: none + auth: none +} + +script:pre-request { + const savedVar = "this works fine"; + console.log(savedVar); +} diff --git a/tests/script-errors/fixtures/collections/script-errors-test/draft-postres-test.bru b/tests/script-errors/fixtures/collections/script-errors-test/draft-postres-test.bru new file mode 100644 index 000000000..1184890aa --- /dev/null +++ b/tests/script-errors/fixtures/collections/script-errors-test/draft-postres-test.bru @@ -0,0 +1,16 @@ +meta { + name: draft-postres-test + type: http + seq: 10 +} + +get { + url: http://localhost:8081/ping + body: none + auth: none +} + +script:post-response { + const savedData = res.body; + console.log(savedData); +} diff --git a/tests/script-errors/fixtures/collections/script-errors-test/draft-tests-test.bru b/tests/script-errors/fixtures/collections/script-errors-test/draft-tests-test.bru new file mode 100644 index 000000000..b93549959 --- /dev/null +++ b/tests/script-errors/fixtures/collections/script-errors-test/draft-tests-test.bru @@ -0,0 +1,16 @@ +meta { + name: draft-tests-test + type: http + seq: 11 +} + +get { + url: http://localhost:8081/ping + body: none + auth: none +} + +tests { + const savedTest = "this works fine"; + console.log(savedTest); +} diff --git a/tests/script-errors/script-errors.spec.ts b/tests/script-errors/script-errors.spec.ts index b0ae373b0..ad1beaf88 100644 --- a/tests/script-errors/script-errors.spec.ts +++ b/tests/script-errors/script-errors.spec.ts @@ -1,28 +1,8 @@ import { test, expect, Page } from '../../playwright'; import { buildScriptErrorLocators, buildCommonLocators } from '../utils/page/locators'; -import { openRequest, closeAllTabs } from '../utils/page/actions'; +import { openRequest, closeAllTabs, sendAndWaitForErrorCard, sendAndWaitForResponse } from '../utils/page/actions'; import { setSandboxMode, runCollection } from '../utils/page/runner'; -/** - * Helper: click send and wait for at least one error card to appear. - */ -const sendAndWaitForErrorCard = async (page: Page) => { - const { request } = buildCommonLocators(page); - const scriptErrorLocators = buildScriptErrorLocators(page); - await request.sendButton().click(); - await scriptErrorLocators.card().waitFor({ state: 'visible', timeout: 15000 }); -}; - -/** - * Helper: click send and wait for a response status code to appear. - * Used for requests that succeed at HTTP level but may have post-response/test errors. - */ -const sendAndWaitForResponse = async (page: Page) => { - const { request, response } = buildCommonLocators(page); - await request.sendButton().click(); - await response.statusCode().waitFor({ state: 'visible', timeout: 15000 }); -}; - /** * Helper: expand a folder in the sidebar and open a nested request. * Clicking the collection row is idempotent (only expands, never collapses). diff --git a/tests/utils/page/actions.ts b/tests/utils/page/actions.ts index edbc8bac0..66d779d73 100644 --- a/tests/utils/page/actions.ts +++ b/tests/utils/page/actions.ts @@ -1,5 +1,5 @@ import { test, expect, Page } from '../../../playwright'; -import { buildCommonLocators } from './locators'; +import { buildCommonLocators, buildScriptErrorLocators } from './locators'; type SandboxMode = 'safe' | 'developer'; @@ -1082,6 +1082,66 @@ const switchWorkspace = async (page: Page, workspaceName: string) => { }); }; +/** + * Navigate to a Script sub-tab (pre-request / post-response) + * @param page - The page object + * @param subTab - The sub-tab to select + */ +const selectScriptSubTab = async (page: Page, subTab: 'pre-request' | 'post-response') => { + await test.step(`Select Script sub-tab "${subTab}"`, async () => { + await selectRequestPaneTab(page, 'Script'); + const trigger = buildCommonLocators(page).paneTabs.tabTrigger(subTab); + await trigger.click(); + await expect(trigger).toContainClass('active'); + }); +}; + +/** + * Clear and type into a CodeMirror editor identified by test ID + * @param page - The page object + * @param editorTestId - The test ID of the editor container + * @param newContent - The content to type + */ +const editCodeMirrorEditor = async (page: Page, editorTestId: string, newContent: string) => { + await test.step(`Edit CodeMirror editor "${editorTestId}"`, async () => { + const locators = buildCommonLocators(page); + const editor = locators.codeMirror.byTestId(editorTestId); + await editor.waitFor({ state: 'visible' }); + const textarea = editor.locator('textarea[tabindex="0"]'); + await textarea.focus(); + const selectAll = process.platform === 'darwin' ? 'Meta+a' : 'Control+a'; + await page.keyboard.press(selectAll); + await page.keyboard.press('Backspace'); + await page.keyboard.type(newContent, { delay: 5 }); + }); +}; + +/** + * Click send and wait for at least one error card to appear. + * @param page - The page object + */ +const sendAndWaitForErrorCard = async (page: Page) => { + await test.step('Send request and wait for error card', async () => { + const { request } = buildCommonLocators(page); + const scriptErrorLocators = buildScriptErrorLocators(page); + await request.sendButton().click(); + await scriptErrorLocators.card().waitFor({ state: 'visible', timeout: 15000 }); + }); +}; + +/** + * Click send and wait for a response status code to appear. + * Used for requests that succeed at HTTP level but may have post-response/test errors. + * @param page - The page object + */ +const sendAndWaitForResponse = async (page: Page) => { + await test.step('Send request and wait for response', async () => { + const { request, response } = buildCommonLocators(page); + await request.sendButton().click(); + await response.statusCode().waitFor({ state: 'visible', timeout: 15000 }); + }); +}; + export { closeAllCollections, openCollection, @@ -1119,7 +1179,11 @@ export { saveRequest, closeAllTabs, createWorkspace, - switchWorkspace + switchWorkspace, + selectScriptSubTab, + editCodeMirrorEditor, + sendAndWaitForErrorCard, + sendAndWaitForResponse }; export type { SandboxMode, EnvironmentType, EnvironmentVariable, ImportCollectionOptions, CreateRequestOptions, CreateUntitledRequestOptions, CreateTransientRequestOptions, AssertionInput }; diff --git a/tests/utils/page/locators.ts b/tests/utils/page/locators.ts index f9d1ea9f6..211d0c947 100644 --- a/tests/utils/page/locators.ts +++ b/tests/utils/page/locators.ts @@ -37,7 +37,8 @@ export const buildCommonLocators = (page: Page) => ({ tabs: { requestTab: (requestName: string) => page.locator('.request-tab .tab-label').filter({ hasText: requestName }), activeRequestTab: () => page.locator('.request-tab.active'), - closeTab: (requestName: string) => page.locator('.request-tab').filter({ hasText: requestName }).getByTestId('request-tab-close-icon') + closeTab: (requestName: string) => page.locator('.request-tab').filter({ hasText: requestName }).getByTestId('request-tab-close-icon'), + draftIndicator: () => page.locator('.request-tab.active .has-changes-icon') }, paneTabs: { responsiveTab: (key: string) => page.getByTestId(`responsive-tab-${key}`), @@ -71,6 +72,9 @@ export const buildCommonLocators = (page: Page) => ({ createEnvButton: () => page.locator('button[id="create-env"]'), envNameInput: () => page.locator('input[name="name"]') }, + codeMirror: { + byTestId: (testId: string) => page.getByTestId(testId).locator('.CodeMirror').first() + }, request: { urlInput: () => page.locator('#request-url .CodeMirror'), urlLine: () => page.locator('#request-url .CodeMirror-line'),