From 646c90819d6618850baa82e33b1c0806aeedbcd6 Mon Sep 17 00:00:00 2001 From: sanish chirayath Date: Fri, 20 Mar 2026 21:36:02 +0530 Subject: [PATCH] feat: enhance ScriptError with source context and remove auto-commenting of untranslated pm commands (#7449) * feat: enhance ScriptError with source context, code snippets, and navigation Co-Authored-By: Claude Opus 4.6 * feat: remove auto-commenting of untranslated pm commands during import Co-Authored-By: Claude Opus 4.6 * feat: update CodeSnippet styles to use theme colors for error and warning highlights * fix: remove unused SCRIPT_TYPES import from network IPC module * refactor: remove unused functions and clean up source-context utility - Removed `getUnifiedScriptContext`, `getWarningSourceGroups`, and related helper functions from `source-context.js` to streamline the utility. - Updated tests in `source-context.spec.js` to reflect the removal of unused functions, ensuring only relevant tests for `findLineInSource` and `getScriptContext` remain. * refactor: simplify ScriptError component and update styles * refactor: streamline tab management in Script components * refactor: enhance tab management in ScriptError and add testsMetadata handling in prepare-request * refactor: improve error source identification in ScriptError component - Enhanced the logic for determining error source types by introducing separate checks for folder and collection files. - Updated the handling of folder file names to ensure accurate UID retrieval and labeling. - Streamlined the overall structure of the getErrorSourceInfo function for better readability and maintainability. * refactor: improve ScriptError component and enhance styling - Simplified the conditions for displaying the ScriptError component in ResponsePane. - Updated navigation logic in ScriptErrorCard to ensure proper handling of source information. - Adjusted styles in StyledWrapper to allow for visible overflow. - Enhanced ScriptErrorIcon to accept additional class names for better styling flexibility. - Minor layout adjustments in RunnerResults ResponsePane for improved UI consistency. * refactor: simplify test description for untranslated pm commands and consolidate error formatter imports * fixes * refactor: update focusedTab logic in Script components to use collection and folder UIDthe respective collection and folder UID instead of the activeTabUid. * feat: add buildErrorContext utility for enhanced error handling in network IPC - Introduced a new utility function `buildErrorContext` to construct detailed error context from script errors. - This function parses error locations, adjusts line numbers, and retrieves relevant source context, improving error reporting. - Removed the previous inline error handling logic from the network IPC module to streamline the codebase. * feat: added playwright test cases to test scriptError behavior * refactor: enhance ScriptError component and improve error handling - Updated labels for error source types in ScriptError to be more concise (e.g., "Request Script" to "Request"). - Improved navigation logic in ScriptErrorCard to ensure proper handling of navigable file paths. - Enhanced styling in StyledWrapper for better visual consistency and user experience. - Added tests to verify fallback behavior for missing error types and non-navigable file paths. - Refined utility functions for better context extraction from script errors. * refactor: normalize file paths for cross-platform compatibility in ScriptError component * refactor: improve file path handling in ScriptError component * refactor: enhance buildErrorContext for improved error handling * docs: add detailed comments to build-error-context for better understanding of error handling * refactor: enhance error block line detection in error formatter - Updated `findScriptBlockEndLine` and `findYmlScriptBlockEndLine` functions to return null for empty or missing blocks, improving accuracy in line detection. - Added comprehensive tests for both functions to ensure correct behavior across various scenarios, including handling of non-.bru and non-.yml files. * refactor: improve error handling and testing in ScriptError and buildErrorContext - Updated ScriptError component to streamline tab management by replacing focusTab with addTab for better request handling. - Enhanced buildErrorContext to return null for empty or missing script blocks in .bru and .yml files, ensuring accurate error reporting. - Added tests to validate behavior for empty script blocks and improved error context extraction in various scenarios. * test: add new tests for script error navigation and handling - Implemented tests for post-response file-path navigation to the Script tab and verification of active sub-tabs. - Added keyboard navigation tests to trigger file-path navigation using the Enter key. - Included a test for multiple error cards to ensure closing one does not affect others. - Enhanced runner tests to verify navigation to the Tests tab from script error results. * refactor: enhance CodeSnippet line rendering and remove unused source-context utilities * refactor: update locators in script-errors tests for improved readability and maintainability * test: enhance script-errors tests to verify error line content * review fixes * refactor: update RunnerResults component and enhance locators for improved testability * refactor: remove buildErrorContext and replace with formatErrorWithContextV2 - Deleted the buildErrorContext function and its associated tests. - Updated network IPC to utilize formatErrorWithContextV2 for improved error context handling. - Enhanced error reporting by ensuring structured error context is returned for desktop UI. * refactor: enhance tab components with data-testid attributes for improved testability - Added data-testid attributes to tab elements in CollectionSettings and FolderSettings components for better integration with testing frameworks. - Updated Tabs and ResponsiveTabs components to include data-testid attributes for tab triggers, enhancing the ability to select and verify tabs in tests. - Modified script-errors tests to utilize new locators for improved readability and maintainability. --------- Co-authored-by: Claude Opus 4.6 --- .../components/CodeSnippet/StyledWrapper.js | 61 +++ .../src/components/CodeSnippet/index.js | 55 +++ .../src/components/CodeSnippet/index.spec.js | 140 ++++++ .../CollectionSettings/Script/index.js | 27 +- .../components/CollectionSettings/index.js | 20 +- .../components/FolderSettings/Script/index.js | 28 +- .../src/components/FolderSettings/index.js | 12 +- .../ResponsePane/ScriptError/StyledWrapper.js | 112 ++++- .../ResponsePane/ScriptError/index.js | 270 ++++++++++- .../ResponsePane/ScriptError/index.spec.js | 242 ++++++++++ .../ResponsePane/ScriptErrorIcon/index.js | 6 +- .../src/components/ResponsePane/index.js | 1 + .../RunnerResults/ResponsePane/index.js | 2 + .../src/components/RunnerResults/index.jsx | 2 +- .../bruno-app/src/components/Tabs/index.js | 1 + .../ReduxStore/slices/collections/index.js | 9 + .../bruno-app/src/ui/ResponsiveTabs/index.js | 1 + .../src/postman/postman-translations.js | 22 +- .../postman-comments.spec.js | 4 +- .../bruno-electron/src/ipc/network/index.js | 40 +- .../src/ipc/network/prepare-request.js | 4 + .../bruno-electron/src/utils/collection.js | 135 +++++- .../tests/utils/collection.spec.js | 191 +++++++- packages/bruno-js/src/index.js | 29 +- .../bruno-js/src/utils/error-formatter.js | 241 ++++++++- .../src/utils/error-formatter.spec.js | 434 +++++++++++++++++ .../collection-script-error/bruno.json | 9 + .../collection-script-error/collection.bru | 8 + .../simple-request.bru | 11 + .../collections/script-errors-test/bruno.json | 9 + .../script-errors-test/collection.bru | 3 + .../error-subfolder/folder-request.bru | 11 + .../error-subfolder/folder.bru | 8 + .../script-errors-test/multiple-errors.bru | 23 + .../post-response-type-error.bru | 17 + .../pre-request-ref-error.bru | 17 + .../script-errors-test/test-script-error.bru | 16 + .../init-user-data/preferences.json | 12 + tests/script-errors/script-errors.spec.ts | 456 ++++++++++++++++++ tests/utils/page/locators.ts | 50 +- 40 files changed, 2609 insertions(+), 130 deletions(-) create mode 100644 packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js create mode 100644 packages/bruno-app/src/components/CodeSnippet/index.js create mode 100644 packages/bruno-app/src/components/CodeSnippet/index.spec.js create mode 100644 packages/bruno-app/src/components/ResponsePane/ScriptError/index.spec.js create mode 100644 tests/script-errors/fixtures/collections/collection-script-error/bruno.json create mode 100644 tests/script-errors/fixtures/collections/collection-script-error/collection.bru create mode 100644 tests/script-errors/fixtures/collections/collection-script-error/simple-request.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/bruno.json create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/collection.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/error-subfolder/folder-request.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/error-subfolder/folder.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/multiple-errors.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/post-response-type-error.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/pre-request-ref-error.bru create mode 100644 tests/script-errors/fixtures/collections/script-errors-test/test-script-error.bru create mode 100644 tests/script-errors/init-user-data/preferences.json create mode 100644 tests/script-errors/script-errors.spec.ts diff --git a/packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js b/packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js new file mode 100644 index 000000000..64b34a53c --- /dev/null +++ b/packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js @@ -0,0 +1,61 @@ +import styled from 'styled-components'; +import { rgba } from 'polished'; + +const StyledWrapper = styled.div` + .code-snippet { + font-family: monospace; + font-size: ${(props) => props.theme.font.size.xs}; + line-height: 1.4; + overflow-x: auto; + border-radius: ${(props) => props.theme.border.radius.base}; + background-color: ${(props) => props.theme.background.elevated}; + border: 1px solid ${(props) => props.theme.border.border2}; + } + + .code-line { + display: flex; + align-items: stretch; + } + + .code-line.highlighted-error { + background-color: ${(props) => rgba(props.theme.colors.text.danger, 0.1)}; + border-left: 3px solid ${(props) => props.theme.colors.text.danger}; + } + + .code-line.highlighted-warning { + background-color: ${(props) => rgba(props.theme.colors.text.warning, 0.1)}; + border-left: 3px solid ${(props) => props.theme.colors.text.warning}; + } + + .code-line:not(.highlighted-error):not(.highlighted-warning) { + border-left: 3px solid transparent; + } + + .code-line-number { + min-width: 2.5rem; + text-align: right; + padding: 0 0.5rem; + color: ${(props) => props.theme.colors.text.muted}; + user-select: none; + flex-shrink: 0; + } + + .code-line-content { + white-space: pre; + padding: 0 0.5rem; + flex: 1; + min-width: 0; + } + + .code-line-separator { + border-left: 3px solid transparent; + } + + .separator-content { + color: ${(props) => props.theme.colors.text.muted}; + user-select: none; + padding: 0 0.5rem; + } +`; + +export default StyledWrapper; diff --git a/packages/bruno-app/src/components/CodeSnippet/index.js b/packages/bruno-app/src/components/CodeSnippet/index.js new file mode 100644 index 000000000..a18f9a8c3 --- /dev/null +++ b/packages/bruno-app/src/components/CodeSnippet/index.js @@ -0,0 +1,55 @@ +import React from 'react'; +import StyledWrapper from './StyledWrapper'; + +const renderLine = (line, highlightClass, hunkIdx) => { + const isHighlighted = line.isHighlighted || line.isError; + const key = hunkIdx != null ? `${hunkIdx}-${line.lineNumber}` : line.lineNumber; + return ( +
+ {line.lineNumber} + + {isHighlighted ? '> ' : ' '}{line.content} + +
+ ); +}; + +const CodeSnippet = ({ lines, hunks, variant = 'error' }) => { + const highlightClass = variant === 'warning' ? 'highlighted-warning' : 'highlighted-error'; + + if (hunks?.length) { + return ( + +
+ {hunks.map((hunk, idx) => ( + + {hunk.hasSeparatorBefore && ( +
+ + {'\u22EE'} +
+ )} + {hunk.lines.map((line) => renderLine(line, highlightClass, idx))} + + ))} +
+ + ); + } + + if (!lines?.length) return null; + + return ( + +
+ {lines.map((line) => renderLine(line, highlightClass))} +
+ + ); +}; + +export default CodeSnippet; diff --git a/packages/bruno-app/src/components/CodeSnippet/index.spec.js b/packages/bruno-app/src/components/CodeSnippet/index.spec.js new file mode 100644 index 000000000..b94aa1c2a --- /dev/null +++ b/packages/bruno-app/src/components/CodeSnippet/index.spec.js @@ -0,0 +1,140 @@ +import '@testing-library/jest-dom'; +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { ThemeProvider } from 'styled-components'; +import CodeSnippet from './index'; + +const theme = { + font: { size: { xs: '0.75rem' } }, + background: { elevated: '#f5f5f5' }, + border: { border2: '#e0e0e0', radius: { base: '4px' } }, + colors: { text: { danger: '#ef4444', warning: '#f59e0b', muted: '#999' } } +}; + +const renderWithTheme = (component) => { + return render( + + {component} + + ); +}; + +const sampleLines = [ + { lineNumber: 3, content: 'const a = 1;', isHighlighted: false }, + { lineNumber: 4, content: 'undefinedVar.foo();', isHighlighted: true }, + { lineNumber: 5, content: 'const b = 2;', isHighlighted: false } +]; + +describe('CodeSnippet', () => { + it('should render nothing when lines is empty', () => { + const { container } = renderWithTheme(); + expect(container.firstChild).toBeNull(); + }); + + it('should render nothing when lines is null', () => { + const { container } = renderWithTheme(); + expect(container.firstChild).toBeNull(); + }); + + it('should render all lines with line numbers', () => { + renderWithTheme(); + expect(screen.getByText('3')).toBeInTheDocument(); + expect(screen.getByText('4')).toBeInTheDocument(); + expect(screen.getByText('5')).toBeInTheDocument(); + }); + + it('should apply error highlight class by default', () => { + const { container } = renderWithTheme(); + const highlightedLine = container.querySelector('.highlighted-error'); + expect(highlightedLine).toBeInTheDocument(); + }); + + it('should apply warning highlight class when variant is warning', () => { + const { container } = renderWithTheme(); + const highlightedLine = container.querySelector('.highlighted-warning'); + expect(highlightedLine).toBeInTheDocument(); + expect(container.querySelector('.highlighted-error')).not.toBeInTheDocument(); + }); + + it('should show > prefix on highlighted line for accessibility', () => { + const { container } = renderWithTheme(); + const codeLineContents = container.querySelectorAll('.code-line-content'); + // The highlighted line (index 1) should start with "> " + expect(codeLineContents[1].textContent).toContain('> '); + // Non-highlighted lines should not have ">" + expect(codeLineContents[0].textContent).not.toContain('>'); + }); + + it('should also support isError property for backward compatibility', () => { + const linesWithIsError = [ + { lineNumber: 1, content: 'line 1', isError: false }, + { lineNumber: 2, content: 'error line', isError: true }, + { lineNumber: 3, content: 'line 3', isError: false } + ]; + const { container } = renderWithTheme(); + expect(container.querySelector('.highlighted-error')).toBeInTheDocument(); + }); + + describe('hunks prop', () => { + const sampleHunks = [ + { + hasSeparatorBefore: false, + lines: [ + { lineNumber: 1, content: 'const a = true;', isHighlighted: false }, + { lineNumber: 2, content: 'pm.vault.get();', isHighlighted: true }, + { lineNumber: 3, content: 'const b = false;', isHighlighted: false } + ] + }, + { + hasSeparatorBefore: true, + lines: [ + { lineNumber: 10, content: 'const x = null;', isHighlighted: false }, + { lineNumber: 11, content: 'pm.cookies.jar();', isHighlighted: true }, + { lineNumber: 12, content: 'const y = undefined;', isHighlighted: false } + ] + } + ]; + + it('should render all lines from all hunks', () => { + renderWithTheme(); + // line numbers + expect(screen.getByText('1')).toBeInTheDocument(); + expect(screen.getByText('2')).toBeInTheDocument(); + expect(screen.getByText('10')).toBeInTheDocument(); + expect(screen.getByText('11')).toBeInTheDocument(); + // content + expect(screen.getByText(/const a = true;/)).toBeInTheDocument(); + expect(screen.getByText(/pm\.vault\.get\(\);/)).toBeInTheDocument(); + expect(screen.getByText(/const x = null;/)).toBeInTheDocument(); + expect(screen.getByText(/pm\.cookies\.jar\(\);/)).toBeInTheDocument(); + }); + + it('should render separator between hunks when hasSeparatorBefore is true', () => { + const { container } = renderWithTheme(); + const separators = container.querySelectorAll('.code-line-separator'); + expect(separators).toHaveLength(1); + // separator should appear between the two hunks, not before the first + const allRows = container.querySelectorAll('.code-line, .code-line-separator'); + const separatorIndex = Array.from(allRows).findIndex((el) => el.classList.contains('code-line-separator')); + // first hunk has 3 lines (indices 0-2), separator should be at index 3 + expect(separatorIndex).toBe(3); + }); + + it('should render the ellipsis character in separator', () => { + const { container } = renderWithTheme(); + const separator = container.querySelector('.separator-content'); + expect(separator.textContent).toBe('\u22EE'); + }); + + it('should apply warning highlights within hunks', () => { + const { container } = renderWithTheme(); + const highlighted = container.querySelectorAll('.highlighted-warning'); + expect(highlighted).toHaveLength(2); + }); + + it('should render nothing when hunks is empty array', () => { + const { container } = renderWithTheme(); + expect(container.firstChild).toBeNull(); + }); + }); +}); diff --git a/packages/bruno-app/src/components/CollectionSettings/Script/index.js b/packages/bruno-app/src/components/CollectionSettings/Script/index.js index 626afd1d5..ae379c01b 100644 --- a/packages/bruno-app/src/components/CollectionSettings/Script/index.js +++ b/packages/bruno-app/src/components/CollectionSettings/Script/index.js @@ -1,9 +1,11 @@ -import React, { useState, useEffect, useRef } from 'react'; +import React, { useEffect, useRef } from 'react'; import get from 'lodash/get'; +import find from 'lodash/find'; import { useDispatch, useSelector } from 'react-redux'; import CodeEditor from 'components/CodeEditor'; import { updateCollectionRequestScript, updateCollectionResponseScript } from 'providers/ReduxStore/slices/collections'; import { saveCollectionSettings } from 'providers/ReduxStore/slices/collections/actions'; +import { updateScriptPaneTab } from 'providers/ReduxStore/slices/tabs'; import { useTheme } from 'providers/Theme'; import { Tabs, TabsList, TabsTrigger, TabsContent } from 'components/Tabs'; import StatusDot from 'components/StatusDot'; @@ -18,27 +20,24 @@ const Script = ({ collection }) => { const requestScript = collection.draft?.root ? get(collection, 'draft.root.request.script.req', '') : get(collection, 'root.request.script.req', ''); const responseScript = collection.draft?.root ? get(collection, 'draft.root.request.script.res', '') : get(collection, 'root.request.script.res', ''); - // Default to post-response if pre-request script is empty - const getInitialTab = () => { + const tabs = useSelector((state) => state.tabs.tabs); + const focusedTab = find(tabs, (t) => t.uid === collection.uid); + const scriptPaneTab = focusedTab?.scriptPaneTab; + + const getDefaultTab = () => { const hasPreRequestScript = requestScript && requestScript.trim().length > 0; return hasPreRequestScript ? 'pre-request' : 'post-response'; }; - const [activeTab, setActiveTab] = useState(getInitialTab); - const prevCollectionUidRef = useRef(collection.uid); + const activeTab = scriptPaneTab || getDefaultTab(); + + const setActiveTab = (tab) => { + dispatch(updateScriptPaneTab({ uid: collection.uid, scriptPaneTab: tab })); + }; const { displayedTheme } = useTheme(); const preferences = useSelector((state) => state.app.preferences); - // Update active tab only when switching to a different collection - useEffect(() => { - if (prevCollectionUidRef.current !== collection.uid) { - prevCollectionUidRef.current = collection.uid; - const hasPreRequestScript = requestScript && requestScript.trim().length > 0; - setActiveTab(hasPreRequestScript ? 'pre-request' : 'post-response'); - } - }, [collection.uid, requestScript]); - // Refresh CodeMirror when tab becomes visible useEffect(() => { const timer = setTimeout(() => { diff --git a/packages/bruno-app/src/components/CollectionSettings/index.js b/packages/bruno-app/src/components/CollectionSettings/index.js index dec9252c0..6343ee4cf 100644 --- a/packages/bruno-app/src/components/CollectionSettings/index.js +++ b/packages/bruno-app/src/components/CollectionSettings/index.js @@ -106,42 +106,42 @@ const CollectionSettings = ({ collection }) => { return (
-
setTab('overview')}> +
setTab('overview')}> Overview
-
setTab('headers')}> +
setTab('headers')}> Headers {activeHeadersCount > 0 && {activeHeadersCount}}
-
setTab('vars')}> +
setTab('vars')}> Vars {activeVarsCount > 0 && {activeVarsCount}}
-
setTab('auth')}> +
setTab('auth')}> Auth {authMode !== 'none' && }
-
setTab('script')}> +
setTab('script')}> Script {hasScripts && }
-
setTab('tests')}> +
setTab('tests')}> Tests {hasTests && }
-
setTab('presets')}> +
setTab('presets')}> Presets {hasPresets && }
-
setTab('proxy')}> +
setTab('proxy')}> Proxy {Object.keys(proxyConfig).length > 0 && proxyEnabled && }
-
setTab('clientCert')}> +
setTab('clientCert')}> Client Certificates {clientCertConfig.length > 0 && }
-
setTab('protobuf')}> +
setTab('protobuf')}> Protobuf {protobufConfig.protoFiles && protobufConfig.protoFiles.length > 0 && }
diff --git a/packages/bruno-app/src/components/FolderSettings/Script/index.js b/packages/bruno-app/src/components/FolderSettings/Script/index.js index 48b282dbe..f3949b095 100644 --- a/packages/bruno-app/src/components/FolderSettings/Script/index.js +++ b/packages/bruno-app/src/components/FolderSettings/Script/index.js @@ -1,9 +1,11 @@ -import React, { useState, useEffect, useRef } from 'react'; +import React, { useEffect, useRef } from 'react'; import get from 'lodash/get'; +import find from 'lodash/find'; import { useDispatch, useSelector } from 'react-redux'; import CodeEditor from 'components/CodeEditor'; import { updateFolderRequestScript, updateFolderResponseScript } from 'providers/ReduxStore/slices/collections'; import { saveFolderRoot } from 'providers/ReduxStore/slices/collections/actions'; +import { updateScriptPaneTab } from 'providers/ReduxStore/slices/tabs'; import { useTheme } from 'providers/Theme'; import { Tabs, TabsList, TabsTrigger, TabsContent } from 'components/Tabs'; import StatusDot from 'components/StatusDot'; @@ -18,27 +20,25 @@ const Script = ({ collection, folder }) => { const requestScript = folder.draft ? get(folder, 'draft.request.script.req', '') : get(folder, 'root.request.script.req', ''); const responseScript = folder.draft ? get(folder, 'draft.request.script.res', '') : get(folder, 'root.request.script.res', ''); - // Default to post-response if pre-request script is empty - const getInitialTab = () => { + const tabs = useSelector((state) => state.tabs.tabs); + const focusedTab = find(tabs, (t) => t.uid === folder.uid); + const scriptPaneTab = focusedTab?.scriptPaneTab; + + // Default to post-response if pre-request script is empty (only when scriptPaneTab is null/undefined) + const getDefaultTab = () => { const hasPreRequestScript = requestScript && requestScript.trim().length > 0; return hasPreRequestScript ? 'pre-request' : 'post-response'; }; - const [activeTab, setActiveTab] = useState(getInitialTab); - const prevFolderUidRef = useRef(folder.uid); + const activeTab = scriptPaneTab || getDefaultTab(); + + const setActiveTab = (tab) => { + dispatch(updateScriptPaneTab({ uid: folder.uid, scriptPaneTab: tab })); + }; const { displayedTheme } = useTheme(); const preferences = useSelector((state) => state.app.preferences); - // Update active tab only when switching to a different folder - useEffect(() => { - if (prevFolderUidRef.current !== folder.uid) { - prevFolderUidRef.current = folder.uid; - const hasPreRequestScript = requestScript && requestScript.trim().length > 0; - setActiveTab(hasPreRequestScript ? 'pre-request' : 'post-response'); - } - }, [folder.uid, requestScript]); - // Refresh CodeMirror when tab becomes visible useEffect(() => { const timer = setTimeout(() => { diff --git a/packages/bruno-app/src/components/FolderSettings/index.js b/packages/bruno-app/src/components/FolderSettings/index.js index e29f6e56e..78a07cfb9 100644 --- a/packages/bruno-app/src/components/FolderSettings/index.js +++ b/packages/bruno-app/src/components/FolderSettings/index.js @@ -77,27 +77,27 @@ const FolderSettings = ({ collection, folder }) => {
-
setTab('headers')}> +
setTab('headers')}> Headers {activeHeadersCount > 0 && {activeHeadersCount}}
-
setTab('script')}> +
setTab('script')}> Script {hasScripts && }
-
setTab('test')}> +
setTab('test')}> Test {hasTests && }
-
setTab('vars')}> +
setTab('vars')}> Vars {activeVarsCount > 0 && {activeVarsCount}}
-
setTab('auth')}> +
setTab('auth')}> Auth {hasAuth && }
-
setTab('docs')}> +
setTab('docs')}> Docs
diff --git a/packages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.js b/packages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.js index 844f51f9b..f05196af2 100644 --- a/packages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.js +++ b/packages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.js @@ -1,44 +1,122 @@ import styled from 'styled-components'; const StyledWrapper = styled.div` - max-height: 200px; - min-height: 70px; - overflow-y: auto; - background-color: ${(props) => props.theme.background.base}; - border: solid 1px ${(props) => props.theme.border.border2}; - border-left: 4px solid ${(props) => props.theme.colors.text.danger}; - border-radius: ${(props) => props.theme.border.radius.base}; - + .script-error-card { + background-color: ${(props) => props.theme.background.base}; + border: solid 1px ${(props) => props.theme.border.border2}; + border-left: 4px solid ${(props) => props.theme.colors.text.danger}; + border-radius: ${(props) => props.theme.border.radius.base}; + padding: 0.75rem 1rem; + display: flex; + flex-direction: column; + gap: 0.5rem; + overflow-y: visible; + } + + .script-error-header { + display: flex; + align-items: center; + justify-content: space-between; + } + .close-button { + all: unset; opacity: 0.7; transition: opacity 0.2s; - + cursor: pointer; + &:hover { opacity: 1; } - + svg { color: ${(props) => props.theme.text}; } } - + .error-title { font-weight: 500; - margin-bottom: 0.375rem; color: ${(props) => props.theme.colors.text.danger}; } - - .error-message { + + .script-error-source-label { + display: flex; + align-items: baseline; + gap: 0.5rem; + min-width: 0; + white-space: nowrap; + font-size: ${(props) => props.theme.font.size.xs}; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.05em; + color: ${(props) => props.theme.colors.text.muted}; + } + + .script-error-file-path { + display: inline-flex; + align-items: center; + gap: 0.25rem; + min-width: 0; + max-width: 100%; + font-family: monospace; + font-size: ${(props) => props.theme.font.size.xs}; + font-weight: 400; + text-transform: none; + letter-spacing: normal; + color: ${(props) => props.theme.colors.text.muted}; + opacity: 0.8; + transition: opacity 0.15s, text-decoration 0.15s; + + span { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + &.navigable { + cursor: pointer; + + &:hover { + opacity: 1; + text-decoration: underline; + } + } + } + + .script-error-message { font-family: monospace; font-size: ${(props) => props.theme.font.size.xs}; line-height: 1.25rem; white-space: pre-wrap; word-break: break-all; - color: ${(props) => props.theme.text}; + color: ${(props) => props.theme.colors.text.danger}; + font-weight: 500; } - .separator { - border-top: 1px solid ${(props) => props.theme.border.border1}; + .script-error-stack-toggle { + all: unset; + display: inline-flex; + align-items: center; + gap: 0.25rem; + cursor: pointer; + font-size: ${(props) => props.theme.font.size.xs}; + color: ${(props) => props.theme.colors.text.muted}; + user-select: none; + + &:hover { + color: ${(props) => props.theme.text}; + } + } + + .script-error-stack { + font-family: monospace; + font-size: ${(props) => props.theme.font.size.xs}; + line-height: 1.4; + color: ${(props) => props.theme.colors.text.muted}; + white-space: pre-wrap; + word-break: break-all; + margin: 0; + padding: 0.25rem 0; } `; diff --git a/packages/bruno-app/src/components/ResponsePane/ScriptError/index.js b/packages/bruno-app/src/components/ResponsePane/ScriptError/index.js index c542fb438..1fee964c1 100644 --- a/packages/bruno-app/src/components/ResponsePane/ScriptError/index.js +++ b/packages/bruno-app/src/components/ResponsePane/ScriptError/index.js @@ -1,37 +1,261 @@ -import React from 'react'; +import React, { useState } from 'react'; +import { useDispatch } from 'react-redux'; +import { IconX, IconChevronDown, IconChevronRight, IconExternalLink } from '@tabler/icons'; import ErrorBanner from 'ui/ErrorBanner'; +import CodeSnippet from 'components/CodeSnippet'; +import { getTreePathFromCollectionToItem } from 'utils/collections'; +import { normalizePath } from 'utils/common/path'; +import { addTab, updateRequestPaneTab, updateScriptPaneTab } from 'providers/ReduxStore/slices/tabs'; +import { updateSettingsSelectedTab, updatedFolderSettingsSelectedTab } from 'providers/ReduxStore/slices/collections'; +import StyledWrapper from './StyledWrapper'; -const ScriptError = ({ item, onClose }) => { +/** + * Determines the source of a script error (request, folder, or collection) + * based on the filePath from the error context. + * + * Bruno executes scripts at three levels in order: collection -> folder -> request. + * When an error occurs, the filePath tells us which level it came from: + * + * filePath: "echo json.bru" -> request-level -> { sourceType: 'request', label: 'Request' } + * filePath: "auth/folder.bru" -> folder-level -> { sourceType: 'folder', label: 'Folder: auth', sourceUid: 'f1' } + * filePath: "collection.bru" -> collection-level -> { sourceType: 'collection', label: 'Collection' } + * + * For folder-level errors, this function walks the tree path from collection to + * the current item to match the folder by its relative path, resolving its UID + * and display name. If the folder can't be matched (e.g. missing tree data), + * it falls back to a generic "Folder" label without a sourceUid. + * + * @param {string|undefined} filePath - Relative path from errorContext (e.g. "subfolder/folder.bru") + * @param {object} item - The current request item + * @param {object} collection - The parent collection (needs .pathname for folder matching) + * @param {function} getTreePath - Function to get the tree path from collection root to item + * @returns {{ sourceType: string, label: string, sourceUid?: string } | null} + */ +const getErrorSourceInfo = (filePath, item, collection, getTreePath) => { + if (!filePath) return null; + + // Normalize backslashes to forward slashes for cross-platform compatibility. + // On Windows, path.relative() produces backslash separators, but the renderer + // logic and regexes expect forward slashes. + const normalizedPath = normalizePath(filePath); + + const isFolderFile = /(?:^|\/)folder\.(?:bru|yml)$/.test(normalizedPath); + const isCollectionFile = normalizedPath === 'collection.bru' || /^opencollection\.yml$/.test(normalizedPath); + + // Folder level (check before collection to avoid folder.yml matching as collection) + if (isFolderFile) { + const info = { sourceType: 'folder', label: 'Folder' }; + const folderFileName = normalizedPath.split('/').pop(); + + // Try to find the folder UID and name from the tree path + if (getTreePath && collection && item) { + const collectionPathname = normalizePath(collection.pathname || ''); + const treePath = getTreePath(collection, item); + if (treePath?.length) { + for (const node of treePath) { + if (node?.type === 'folder') { + const nodePath = normalizePath(node.pathname || ''); + const folderRelPath = nodePath && nodePath.startsWith(collectionPathname) + ? nodePath.slice(collectionPathname.length).replace(/^\//, '') + '/' + folderFileName + : folderFileName; + if (folderRelPath === normalizedPath) { + info.sourceUid = node.uid; + info.label = `Folder: ${node.name}`; + break; + } + } + } + } + } + + return info; + } + + // Collection level + if (isCollectionFile) { + return { sourceType: 'collection', label: 'Collection' }; + } + + // Request level + return { sourceType: 'request', label: 'Request' }; +}; + +const ScriptErrorCard = ({ title, message, errorContext, item, collection, scriptPhase, onClose }) => { + const dispatch = useDispatch(); + const [showStack, setShowStack] = useState(false); + + const displayFilePath = errorContext?.filePath ? normalizePath(errorContext.filePath) : null; + + const sourceInfo = getErrorSourceInfo( + errorContext?.filePath, + item, + collection, + getTreePathFromCollectionToItem + ); + + const canNavigate = sourceInfo + && collection?.uid + && (sourceInfo.sourceType === 'collection' + || (sourceInfo.sourceType === 'folder' && sourceInfo.sourceUid) + || (sourceInfo.sourceType === 'request' && item?.uid)); + + const handleNavigateKeyDown = (e) => { + if (!canNavigate) return; + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handleNavigate(); + } + }; + + const handleNavigate = () => { + if (!canNavigate) return; + + // CollectionSettings expects 'tests', FolderSettings expects 'test' + const collectionSettingsTab = scriptPhase === 'test' ? 'tests' : 'script'; + const folderSettingsTab = scriptPhase === 'test' ? 'test' : 'script'; + + if (sourceInfo.sourceType === 'collection') { + dispatch(addTab({ uid: collection.uid, collectionUid: collection.uid, type: 'collection-settings' })); + dispatch(updateSettingsSelectedTab({ collectionUid: collection.uid, tab: collectionSettingsTab })); + if (collectionSettingsTab === 'script') { + dispatch(updateScriptPaneTab({ uid: collection.uid, scriptPaneTab: scriptPhase })); + } + } else if (sourceInfo.sourceType === 'folder' && sourceInfo.sourceUid) { + dispatch(addTab({ uid: sourceInfo.sourceUid, collectionUid: collection.uid, type: 'folder-settings' })); + dispatch(updatedFolderSettingsSelectedTab({ collectionUid: collection.uid, folderUid: sourceInfo.sourceUid, tab: folderSettingsTab })); + if (folderSettingsTab === 'script') { + dispatch(updateScriptPaneTab({ uid: sourceInfo.sourceUid, scriptPaneTab: scriptPhase })); + } + } else if (sourceInfo.sourceType === 'request') { + dispatch(addTab({ uid: item.uid, collectionUid: collection.uid, type: 'request' })); + if (scriptPhase === 'test') { + dispatch(updateRequestPaneTab({ uid: item.uid, requestPaneTab: 'tests' })); + } else { + dispatch(updateRequestPaneTab({ uid: item.uid, requestPaneTab: 'script' })); + dispatch(updateScriptPaneTab({ uid: item.uid, scriptPaneTab: scriptPhase })); + } + } + }; + + if (!errorContext) { + return ; + } + + return ( + +
+
+
{title}
+ {onClose && ( + + )} +
+ {(sourceInfo || displayFilePath) && ( +
+ {sourceInfo && {sourceInfo.label}} + {displayFilePath && ( + + {displayFilePath} + {canNavigate && } + + )} +
+ )} + +
+ {errorContext.errorType || 'Error'}: {message} +
+ {errorContext.stack && ( +
+ + {showStack && ( + 
{errorContext.stack}
+ )} +
+ )} +
+ + ); +}; + +const ScriptError = ({ item, collection, onClose }) => { const preRequestError = item?.preRequestScriptErrorMessage; const postResponseError = item?.postResponseScriptErrorMessage; const testScriptError = item?.testScriptErrorMessage; if (!preRequestError && !postResponseError && !testScriptError) return null; - const errors = []; + const preRequestContext = item?.preRequestScriptErrorContext; + const postResponseContext = item?.postResponseScriptErrorContext; + const testContext = item?.testScriptErrorContext; - if (preRequestError) { - errors.push({ - title: 'Pre-Request Script Error', - message: preRequestError - }); + const hasAnyContext = preRequestContext || postResponseContext || testContext; + + // If no error context available for any error, fall back to ErrorBanner + if (!hasAnyContext) { + const errors = []; + if (preRequestError) errors.push({ title: 'Pre-Request Script Error', message: preRequestError }); + if (postResponseError) errors.push({ title: 'Post-Response Script Error', message: postResponseError }); + if (testScriptError) errors.push({ title: 'Test Script Error', message: testScriptError }); + return ; } - if (postResponseError) { - errors.push({ - title: 'Post-Response Script Error', - message: postResponseError - }); - } - - if (testScriptError) { - errors.push({ - title: 'Test Script Error', - message: testScriptError - }); - } - - return ; + return ( +
+ {preRequestError && ( + + )} + {postResponseError && ( + + )} + {testScriptError && ( + + )} +
+ ); }; export default ScriptError; diff --git a/packages/bruno-app/src/components/ResponsePane/ScriptError/index.spec.js b/packages/bruno-app/src/components/ResponsePane/ScriptError/index.spec.js new file mode 100644 index 000000000..3324d64d1 --- /dev/null +++ b/packages/bruno-app/src/components/ResponsePane/ScriptError/index.spec.js @@ -0,0 +1,242 @@ +import '@testing-library/jest-dom'; +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { ThemeProvider } from 'styled-components'; +import { Provider } from 'react-redux'; +import { configureStore } from '@reduxjs/toolkit'; +import ScriptError from './index'; + +const theme = { + font: { size: { xs: '0.75rem' } }, + text: '#333', + background: { base: '#fff', elevated: '#f5f5f5' }, + border: { border1: '#e0e0e0', border2: '#d0d0d0', radius: { base: '4px' } }, + colors: { text: { danger: '#ef4444', warning: '#f59e0b', muted: '#999' } } +}; + +const mockStore = configureStore({ + reducer: { + tabs: (state = { tabs: [], activeTabUid: null }) => state, + collections: (state = { collections: [] }) => state + } +}); + +const renderWithProviders = (component) => { + return render( + + + {component} + + + ); +}; + +const mockCollection = { + uid: 'col-1', + pathname: '/home/user/collection' +}; + +const mockErrorContext = { + errorType: 'ReferenceError', + filePath: 'echo json.bru', + errorLine: 4, + lines: [ + { lineNumber: 3, content: 'const data = res.body;', isError: false }, + { lineNumber: 4, content: 'console.log(undefinedVar);', isError: true }, + { lineNumber: 5, content: '', isError: false } + ], + stack: ' at echo json.bru:4:5' +}; + +describe('ScriptError', () => { + it('should render nothing when no errors', () => { + const { container } = renderWithProviders(); + expect(container.firstChild).toBeNull(); + }); + + it('should fall back to ErrorBanner when no errorContext', () => { + const item = { + preRequestScriptErrorMessage: 'something broke' + }; + renderWithProviders(); + expect(screen.getByText('Pre-Request Script Error')).toBeInTheDocument(); + expect(screen.getByText('something broke')).toBeInTheDocument(); + }); + + it('should show CodeSnippet when errorContext is available', () => { + const item = { + preRequestScriptErrorMessage: 'undefinedVar is not defined', + preRequestScriptErrorContext: mockErrorContext + }; + const { container } = renderWithProviders(); + expect(screen.getByText('Pre-Request Script Error')).toBeInTheDocument(); + expect(container.querySelector('.code-snippet')).toBeInTheDocument(); + }); + + it('should show error line highlighted', () => { + const item = { + preRequestScriptErrorMessage: 'undefinedVar is not defined', + preRequestScriptErrorContext: mockErrorContext + }; + const { container } = renderWithProviders(); + expect(container.querySelector('.highlighted-error')).toBeInTheDocument(); + }); + + it('should show error type and message', () => { + const item = { + preRequestScriptErrorMessage: 'undefinedVar is not defined', + preRequestScriptErrorContext: mockErrorContext + }; + renderWithProviders(); + expect(screen.getByText('ReferenceError: undefinedVar is not defined')).toBeInTheDocument(); + }); + + it('should show file path with source label', () => { + const item = { + preRequestScriptErrorMessage: 'undefinedVar is not defined', + preRequestScriptErrorContext: mockErrorContext + }; + const { container } = renderWithProviders(); + expect(container.querySelector('.script-error-file-path')).toBeInTheDocument(); + expect(screen.getByText('echo json.bru')).toBeInTheDocument(); + expect(screen.getByText('Request')).toBeInTheDocument(); + }); + + it('should show "Collection Script" label for collection-level errors', () => { + const item = { + preRequestScriptErrorMessage: 'collection error', + preRequestScriptErrorContext: { + ...mockErrorContext, + filePath: 'collection.bru' + } + }; + renderWithProviders(); + expect(screen.getByText('Collection')).toBeInTheDocument(); + expect(screen.getByText('collection.bru')).toBeInTheDocument(); + }); + + it('should show "Folder Script" label for folder-level errors', () => { + const item = { + preRequestScriptErrorMessage: 'folder error', + preRequestScriptErrorContext: { + ...mockErrorContext, + filePath: 'subfolder/folder.bru' + } + }; + renderWithProviders(); + expect(screen.getByText('Folder')).toBeInTheDocument(); + expect(screen.getByText('subfolder/folder.bru')).toBeInTheDocument(); + }); + + it('should show "Request Script" label for request-level errors', () => { + const item = { + postResponseScriptErrorMessage: 'request error', + postResponseScriptErrorContext: { + ...mockErrorContext, + filePath: 'my-request.bru' + } + }; + renderWithProviders(); + expect(screen.getByText('Request')).toBeInTheDocument(); + expect(screen.getByText('my-request.bru')).toBeInTheDocument(); + }); + + it('should toggle stack trace visibility', () => { + const item = { + preRequestScriptErrorMessage: 'undefinedVar is not defined', + preRequestScriptErrorContext: mockErrorContext + }; + renderWithProviders(); + + // Stack should be hidden by default + expect(screen.queryByText(/at echo json\.bru/)).not.toBeInTheDocument(); + expect(screen.getByText('Show stack trace')).toBeInTheDocument(); + + // Click to show + fireEvent.click(screen.getByText('Show stack trace')); + expect(screen.getByText(/at echo json\.bru/)).toBeInTheDocument(); + expect(screen.getByText('Hide stack trace')).toBeInTheDocument(); + + // Click to hide + fireEvent.click(screen.getByText('Hide stack trace')); + expect(screen.queryByText(/at echo json\.bru/)).not.toBeInTheDocument(); + }); + + it('should call onClose when close button is clicked', () => { + const onClose = jest.fn(); + const item = { + preRequestScriptErrorMessage: 'error', + preRequestScriptErrorContext: mockErrorContext + }; + const { container } = renderWithProviders(); + const closeButton = container.querySelector('.close-button'); + fireEvent.click(closeButton); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('should fallback to "Error" when errorType is missing', () => { + const item = { + preRequestScriptErrorMessage: 'something went wrong', + preRequestScriptErrorContext: { + ...mockErrorContext, + errorType: undefined + } + }; + renderWithProviders(); + expect(screen.getByText('Error: something went wrong')).toBeInTheDocument(); + }); + + it('should not show pointer cursor on non-navigable file path', () => { + const item = { + preRequestScriptErrorMessage: 'error', + preRequestScriptErrorContext: mockErrorContext + }; + // No item.uid means request-level navigation is disabled + const { container } = renderWithProviders(); + const filePath = container.querySelector('.script-error-file-path'); + expect(filePath).not.toHaveClass('navigable'); + }); + + it('should detect folder-level errors with Windows backslash paths', () => { + const item = { + preRequestScriptErrorMessage: 'folder error', + preRequestScriptErrorContext: { + ...mockErrorContext, + filePath: 'subfolder\\folder.bru' + } + }; + renderWithProviders(); + expect(screen.getByText('Folder')).toBeInTheDocument(); + expect(screen.getByText('subfolder/folder.bru')).toBeInTheDocument(); + }); + + it('should detect request-level errors with Windows backslash paths', () => { + const item = { + postResponseScriptErrorMessage: 'request error', + postResponseScriptErrorContext: { + ...mockErrorContext, + filePath: 'subfolder\\my-request.bru' + } + }; + renderWithProviders(); + expect(screen.getByText('Request')).toBeInTheDocument(); + expect(screen.getByText('subfolder/my-request.bru')).toBeInTheDocument(); + }); + + it('should handle multiple errors with their own context', () => { + const item = { + preRequestScriptErrorMessage: 'pre error', + preRequestScriptErrorContext: mockErrorContext, + testScriptErrorMessage: 'test error', + testScriptErrorContext: { + ...mockErrorContext, + errorType: 'TypeError' + } + }; + renderWithProviders(); + expect(screen.getByText('Pre-Request Script Error')).toBeInTheDocument(); + expect(screen.getByText('Test Script Error')).toBeInTheDocument(); + expect(screen.getByText('ReferenceError: pre error')).toBeInTheDocument(); + expect(screen.getByText('TypeError: test error')).toBeInTheDocument(); + }); +}); diff --git a/packages/bruno-app/src/components/ResponsePane/ScriptErrorIcon/index.js b/packages/bruno-app/src/components/ResponsePane/ScriptErrorIcon/index.js index 41420eeb0..0c231aff9 100644 --- a/packages/bruno-app/src/components/ResponsePane/ScriptErrorIcon/index.js +++ b/packages/bruno-app/src/components/ResponsePane/ScriptErrorIcon/index.js @@ -1,15 +1,17 @@ import React from 'react'; +import classnames from 'classnames'; import { IconAlertCircle } from '@tabler/icons'; import ToolHint from 'components/ToolHint'; -const ScriptErrorIcon = ({ itemUid, onClick }) => { +const ScriptErrorIcon = ({ itemUid, onClick, className }) => { const toolhintId = `script-error-icon-${itemUid}`; return ( <>
diff --git a/packages/bruno-app/src/components/ResponsePane/index.js b/packages/bruno-app/src/components/ResponsePane/index.js index 450c798c9..617f862dc 100644 --- a/packages/bruno-app/src/components/ResponsePane/index.js +++ b/packages/bruno-app/src/components/ResponsePane/index.js @@ -304,6 +304,7 @@ const ResponsePane = ({ item, collection }) => { setShowScriptErrorCard(false)} + collection={collection} /> )}
diff --git a/packages/bruno-app/src/components/RunnerResults/ResponsePane/index.js b/packages/bruno-app/src/components/RunnerResults/ResponsePane/index.js index 7aedf12f9..12656418d 100644 --- a/packages/bruno-app/src/components/RunnerResults/ResponsePane/index.js +++ b/packages/bruno-app/src/components/RunnerResults/ResponsePane/index.js @@ -120,6 +120,7 @@ const ResponsePane = ({ rightPaneWidth, item, collection }) => {
{hasScriptError && !showScriptErrorCard && ( setShowScriptErrorCard(true)} /> @@ -134,6 +135,7 @@ const ResponsePane = ({ rightPaneWidth, item, collection }) => { setShowScriptErrorCard(false)} + collection={collection} /> )}
diff --git a/packages/bruno-app/src/components/RunnerResults/index.jsx b/packages/bruno-app/src/components/RunnerResults/index.jsx index 622555145..d6d4e25f5 100644 --- a/packages/bruno-app/src/components/RunnerResults/index.jsx +++ b/packages/bruno-app/src/components/RunnerResults/index.jsx @@ -425,7 +425,7 @@ export default function RunnerResults({ collection }) { {filteredItems.map((item) => { return (
-
+
{allTestsPassed(item) diff --git a/packages/bruno-app/src/components/Tabs/index.js b/packages/bruno-app/src/components/Tabs/index.js index 15f4890be..78547572c 100644 --- a/packages/bruno-app/src/components/Tabs/index.js +++ b/packages/bruno-app/src/components/Tabs/index.js @@ -23,6 +23,7 @@ export const TabsTrigger = ({ value: triggerValue, children, className = '' }) = return (