feat: enhance ScriptError with source context and remove auto-commenting of untranslated pm commands#7449
Merged
bijin-bruno merged 30 commits intousebruno:mainfrom Mar 20, 2026
Conversation
sanish-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds UI and tooling to capture, map, and display script execution errors: new CodeSnippet UI, source-context utilities, error-context builder in Electron IPC, Redux-backed per-script error contexts, script-wrapping with metadata, and tests/fixtures to exercise end-to-end behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Exec as Script Execution
participant IPC as Electron IPC
participant Store as Redux Store
participant UI as Renderer (ResponsePane)
participant Snip as CodeSnippet
Exec->>IPC: script throws error (pre/post/test)
IPC->>IPC: buildErrorContext(error, scriptType, itemPathname, collectionPath, scriptMetadata)
IPC->>Store: dispatch scriptExecutionResult(..., errorContext)
Store->>UI: state updated with errorContext
UI->>UI: use getScriptContext / findLineInSource to prepare lines/hunks
UI->>Snip: render lines/hunks with highlighted line
Snip-->>UI: rendered snippet displayed in ScriptErrorCard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
sanish-bruno
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/ResponsePane/index.js (1)
231-236:⚠️ Potential issue | 🟠 MajorClicking the error icon off the Response tab now hides it without showing the card.
The icon still renders on other tabs, but the card only renders when
responsePaneTab === 'response'. On headers/timeline/tests, the click just flipsshowScriptErrorCardtotrue, the icon disappears, and nothing is shown until the user manually switches tabs. Either auto-switch to the Response tab in the click handler or only render the icon there.Suggested fix
{hasScriptError && !showScriptErrorCard && ( <ScriptErrorIcon itemUid={item.uid} - onClick={() => setShowScriptErrorCard(true)} + onClick={() => { + if (focusedTab?.responsePaneTab !== 'response') { + selectTab('response'); + } + setShowScriptErrorCard(true); + }} /> )}Also applies to: 303-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ResponsePane/index.js` around lines 231 - 236, The ScriptErrorIcon click currently only sets showScriptErrorCard true but the card only renders when responsePaneTab === 'response', causing the icon to disappear on other tabs; update the click behavior in the ScriptErrorIcon block (where hasScriptError, showScriptErrorCard, setShowScriptErrorCard, responsePaneTab and item.uid are used) to either (A) also set responsePaneTab to 'response' when calling setShowScriptErrorCard(true) so the card becomes visible immediately, or (B) restrict rendering of ScriptErrorIcon to when responsePaneTab === 'response' so the icon never appears on non-response tabs—pick one approach and apply the same change to the other occurrence (lines ~303-308).
🧹 Nitpick comments (3)
packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js (1)
19-26: Use theme-driven highlight backgrounds here.The left border is theme-aware, but the red/amber background tints are hardcoded. That will drift from dark/custom themes and makes this wrapper inconsistent with the rest of the styled-components setup. Please route these fills through theme tokens as well.
Based on learnings, "Use styled component's theme prop to manage CSS colors, not CSS variables, when in the context of a styled component or any React component using styled components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js` around lines 19 - 26, The highlight backgrounds for .code-line.highlighted-error and .code-line.highlighted-warning are hardcoded rgba values; update both rules to use theme tokens instead of literal colors (similar to how the border-left uses props.theme.colors.text.danger / .warning). Replace the rgba(...) background with a theme-driven property (e.g., props.theme.colors.background.danger / props.theme.colors.background.warning or a named token like props.theme.colors.highlight.error / .warning), and if those tokens don’t exist add them to the theme rather than introducing inline color literals in StyledWrapper.js.packages/bruno-electron/src/ipc/network/index.js (2)
11-11: Unused import:SCRIPT_TYPES.
SCRIPT_TYPESis imported but not used anywhere in this file.Proposed fix
-const { VarsRuntime, AssertRuntime, ScriptRuntime, TestRuntime, SCRIPT_TYPES } = require('@usebruno/js'); +const { VarsRuntime, AssertRuntime, ScriptRuntime, TestRuntime } = require('@usebruno/js');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/index.js` at line 11, The import statement destructures SCRIPT_TYPES but it is unused; update the require call in this module (where VarsRuntime, AssertRuntime, ScriptRuntime, TestRuntime, SCRIPT_TYPES are imported) to remove SCRIPT_TYPES from the destructuring so only VarsRuntime, AssertRuntime, ScriptRuntime, and TestRuntime are imported, or if SCRIPT_TYPES is actually needed elsewhere, reference it where required; in short, eliminate the unused SCRIPT_TYPES symbol from the import to fix the unused-import warning.
520-522: Empty catch block silently swallows errors.If
buildErrorContextfails, returningnullis fine for graceful degradation, but consider logging the error for debugging purposes during development.Proposed fix
- } catch { + } catch (err) { + console.debug('buildErrorContext failed:', err); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/index.js` around lines 520 - 522, The catch block that swallows errors when calling buildErrorContext should capture the thrown error and log it before returning null; update the anonymous catch to use a parameter (e.g., catch (err)) and call the project's logger (or console.error) with context identifying buildErrorContext and the error (for example using processLogger.error or console.error) and then return null so behavior is unchanged but failures are visible during debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/CodeSnippet/index.js`:
- Around line 4-17: The renderLine function currently uses line.lineNumber as
the React key (renderLine), which can collide across different hunks; update
renderLine to accept a key prefix or hunk index (e.g., add a parameter keyPrefix
or hunkIndex) and use a composite key like `${keyPrefix}-${line.lineNumber}` (or
`${hunkIndex}-${line.lineNumber}`) when returning the element; then update all
call sites that invoke renderLine to pass the hunk-specific prefix/index so each
key is unique across hunks.
In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js`:
- Around line 75-77: sourceInfo.sourceUid can be undefined causing addTab({ uid:
sourceInfo.sourceUid, ... }) to dispatch an invalid tab; add a guard before
dispatching in the folder branch: check sourceInfo.sourceUid (or derive a safe
fallback) and only call addTab and updatedFolderSettingsSelectedTab when a valid
UID exists, otherwise handle gracefully (e.g., log/dispatch an error, open a
default collection tab, or no-op). Update the code around the folder branch that
calls addTab and updatedFolderSettingsSelectedTab to validate
sourceInfo.sourceUid first and use a clear fallback behavior.
- Around line 34-38: The folderRelPath computation uses manual string ops on
node.pathname and collectionPathname and therefore fails on Windows; replace
this logic to call the existing getRelativePath utility from
packages/bruno-app/src/utils/common/path.js (use
getRelativePath(collectionPathname, node.pathname) or similar API) to compute
the relative path, then append '/folder.bru' in a platform-safe way (or use
path.join style if needed); update the code that assigns folderRelPath (the
variable computed from node.pathname and collectionPathname) to use
getRelativePath instead of startsWith/slice/replace.
In `@packages/bruno-app/src/utils/source-context.js`:
- Around line 121-127: getRelativePath currently assumes POSIX separators and
BRU filenames by using startsWith and manual slicing, which breaks on Windows
and for YAML/open-collection formats; update getRelativePath (and the similar
logic around lines 151-181) to build and compare paths using the shared path
utilities (or Node's path/path.posix modules) and the collection's
format/extension instead of hardcoded '/' or '.bru' strings: compute the
collection base/extension from the collection metadata, normalize both
collectionPathname and fullPath with the shared normalize/join helpers (or
path.resolve/path.relative) and then derive the relative path via path.relative
(or the shared helper) so OS separators and different collection filename
extensions (e.g., opencollection.yml) are handled correctly. Ensure you
import/use the existing shared path util functions rather than manual string
ops.
- Around line 6-10: stripCommentsAndStrings currently removes entire template
literals including their ${...} expression bodies, losing attributeable calls;
modify stripCommentsAndStrings so backtick template handling preserves ${...}
contents (while still blanking out literal text and escapes). Implement a
template-aware parser inside stripCommentsAndStrings that, when encountering `
starts a template mode that replaces literal characters with spaces but switches
to an expression mode when it sees ${, copying the expression contents verbatim
(handling nested braces and escapes) until the matching } returns to template
mode, and only ends the template on the closing backtick; keep existing handling
for // and /* */ and single/double-quoted strings.
In `@packages/bruno-converters/src/postman/postman-translations.js`:
- Around line 122-134: The current flow returns translatedScript from
translateCode(processedScript) which may produce mixed JS that still references
pm.* or postman.*; instead, after calling translateCode (and also after
processRegexReplacement fallback), scan translatedScript for any pm\.|postman\.
usages and transform/comment those lines (e.g., via a new helper like
commentOutPmUsage or sanitizePmReferences) so imported scripts never execute
unresolved pm/postman objects; update the logic around translateCode,
processRegexReplacement and the returned translatedScript to run that sanitizer
and return the fully commented/annotated script when any pm/postman tokens
remain.
In `@packages/bruno-js/src/utils/error-formatter.js`:
- Around line 67-96: The current findScriptBlockEndLine treats the first line
that starts with `}` as the end of the script block; change it to track brace
nesting instead: when pattern in findScriptBlockEndLine (BLOCK_PATTERNS,
pattern) matches start of the block, set a depth counter and for each subsequent
line update depth by counting occurrences of `{` and `}` (e.g., depth += opens -
closes) and only treat the block as closed when the cumulative depth indicates
the outer block is closed (depth goes negative / indicates the initial opening
was balanced); update uses of inBlock and result and keep caching via cacheKey
unchanged.
---
Outside diff comments:
In `@packages/bruno-app/src/components/ResponsePane/index.js`:
- Around line 231-236: The ScriptErrorIcon click currently only sets
showScriptErrorCard true but the card only renders when responsePaneTab ===
'response', causing the icon to disappear on other tabs; update the click
behavior in the ScriptErrorIcon block (where hasScriptError,
showScriptErrorCard, setShowScriptErrorCard, responsePaneTab and item.uid are
used) to either (A) also set responsePaneTab to 'response' when calling
setShowScriptErrorCard(true) so the card becomes visible immediately, or (B)
restrict rendering of ScriptErrorIcon to when responsePaneTab === 'response' so
the icon never appears on non-response tabs—pick one approach and apply the same
change to the other occurrence (lines ~303-308).
---
Nitpick comments:
In `@packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js`:
- Around line 19-26: The highlight backgrounds for .code-line.highlighted-error
and .code-line.highlighted-warning are hardcoded rgba values; update both rules
to use theme tokens instead of literal colors (similar to how the border-left
uses props.theme.colors.text.danger / .warning). Replace the rgba(...)
background with a theme-driven property (e.g.,
props.theme.colors.background.danger / props.theme.colors.background.warning or
a named token like props.theme.colors.highlight.error / .warning), and if those
tokens don’t exist add them to the theme rather than introducing inline color
literals in StyledWrapper.js.
In `@packages/bruno-electron/src/ipc/network/index.js`:
- Line 11: The import statement destructures SCRIPT_TYPES but it is unused;
update the require call in this module (where VarsRuntime, AssertRuntime,
ScriptRuntime, TestRuntime, SCRIPT_TYPES are imported) to remove SCRIPT_TYPES
from the destructuring so only VarsRuntime, AssertRuntime, ScriptRuntime, and
TestRuntime are imported, or if SCRIPT_TYPES is actually needed elsewhere,
reference it where required; in short, eliminate the unused SCRIPT_TYPES symbol
from the import to fix the unused-import warning.
- Around line 520-522: The catch block that swallows errors when calling
buildErrorContext should capture the thrown error and log it before returning
null; update the anonymous catch to use a parameter (e.g., catch (err)) and call
the project's logger (or console.error) with context identifying
buildErrorContext and the error (for example using processLogger.error or
console.error) and then return null so behavior is unchanged but failures are
visible during debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d6197dd-26ff-42fe-a250-3ac52ebdfdac
📒 Files selected for processing (17)
packages/bruno-app/src/components/CodeSnippet/StyledWrapper.jspackages/bruno-app/src/components/CodeSnippet/index.jspackages/bruno-app/src/components/CodeSnippet/index.spec.jspackages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/ScriptError/index.jspackages/bruno-app/src/components/ResponsePane/ScriptError/index.spec.jspackages/bruno-app/src/components/ResponsePane/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/utils/source-context.jspackages/bruno-app/src/utils/source-context.spec.jspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-converters/src/postman/postman-translations.jspackages/bruno-converters/tests/postman/postman-translations/postman-comments.spec.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/utils/collection.jspackages/bruno-electron/tests/utils/collection.spec.jspackages/bruno-js/src/utils/error-formatter.js
Comment on lines
+34
to
+38
| const folderRelPath = node.pathname | ||
| ? (node.pathname.startsWith(collectionPathname) | ||
| ? node.pathname.slice(collectionPathname.length).replace(/^\//, '') + '/folder.bru' | ||
| : 'folder.bru') | ||
| : 'folder.bru'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if this file or related utils use path module for cross-platform handling
rg -n "require\('path'\)|from 'path'" packages/bruno-app/src/components/ResponsePane/ScriptError/
rg -n "path\.sep|path\.join|path\.normalize" packages/bruno-app/src/components/ResponsePane/ScriptError/Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Read the file to understand context
cat -n packages/bruno-app/src/components/ResponsePane/ScriptError/index.js | head -60Repository: usebruno/bruno
Length of output: 2810
🏁 Script executed:
# Search for where node.pathname comes from and how it's used
rg -B 10 -A 5 "node.pathname" packages/bruno-app/src/components/ResponsePane/ScriptError/Repository: usebruno/bruno
Length of output: 2165
🏁 Script executed:
# Check how the codebase handles path operations in similar contexts
rg -n "pathname" packages/bruno-app/src/components/ResponsePane/ -A 2 -B 2Repository: usebruno/bruno
Length of output: 3230
🏁 Script executed:
# Look for path handling patterns across the codebase to understand conventions
rg "startsWith.*\/" packages/bruno-app/src/ -t js -t jsx -t ts -t tsx | head -20Repository: usebruno/bruno
Length of output: 150
🏁 Script executed:
# Find where pathname is set in node/folder objects
rg -n "\.pathname\s*=" packages/bruno-app/src/ -t js -t jsx -t ts -t tsx --max-count=30Repository: usebruno/bruno
Length of output: 85
🏁 Script executed:
# Check the getTreePath implementation
rg -n "getTreePath" packages/bruno-app/src/utils/ -A 10 -B 2Repository: usebruno/bruno
Length of output: 10969
🏁 Script executed:
# Look for pathname generation to understand if it's using path.join or hardcoded slashes
rg -n "pathname" packages/bruno-app/src/utils/collections -B 3 -A 3Repository: usebruno/bruno
Length of output: 4622
🏁 Script executed:
# Find getRelativePath implementation
rg -n "getRelativePath" packages/bruno-app/src/utils/ -A 8Repository: usebruno/bruno
Length of output: 18010
🏁 Script executed:
# Check where pathname is populated/created in the data structure
rg -n "pathname\s*:" packages/bruno-app/src/ -t js | head -40Repository: usebruno/bruno
Length of output: 4414
🏁 Script executed:
# Look for how collection pathname is set
rg -n "collection\.pathname" packages/bruno-app/src/ipc/ -B 3 -A 3Repository: usebruno/bruno
Length of output: 125
🏁 Script executed:
# Find where pathname values are created
rg -n "pathname.*=" packages/bruno-app/src/providers/ReduxStore/slices/collections/ -A 2 | head -40Repository: usebruno/bruno
Length of output: 4134
🏁 Script executed:
# Check the main process IPC handlers
fd -t f "ipc" packages/ | grep -v node_modules | head -10Repository: usebruno/bruno
Length of output: 153
🏁 Script executed:
# Look at how pathname is handled in the electron main process
rg -n "pathname" packages/bruno-electron/src/ -A 2 -B 2 | head -50Repository: usebruno/bruno
Length of output: 4748
🏁 Script executed:
# Check if source-context.js uses the proper path utilities
head -20 packages/bruno-app/src/utils/source-context.jsRepository: usebruno/bruno
Length of output: 708
🏁 Script executed:
# Verify the getRelativePath implementation in common/path.js
cat -n packages/bruno-app/src/utils/common/path.js | sed -n '75,95p'Repository: usebruno/bruno
Length of output: 801
Use cross-platform path handling instead of hardcoded forward slashes.
Lines 34-38 use hardcoded forward slash checks (startsWith, slice, replace(/^\//)), which will fail on Windows where node.pathname may contain backslashes. Import and use the getRelativePath utility from packages/bruno-app/src/utils/common/path.js instead of manual string manipulation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js` around
lines 34 - 38, The folderRelPath computation uses manual string ops on
node.pathname and collectionPathname and therefore fails on Windows; replace
this logic to call the existing getRelativePath utility from
packages/bruno-app/src/utils/common/path.js (use
getRelativePath(collectionPathname, node.pathname) or similar API) to compute
the relative path, then append '/folder.bru' in a platform-safe way (or use
path.join style if needed); update the code that assigns folderRelPath (the
variable computed from node.pathname and collectionPathname) to use
getRelativePath instead of startsWith/slice/replace.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/bruno-app/src/components/ResponsePane/ScriptError/index.js (1)
98-129:⚠️ Potential issue | 🟠 MajorThese new controls aren't keyboard accessible.
The close affordance, source-path navigation, and stack toggle are all plain
div/spanelements withonClick. They aren't focusable or operable from the keyboard, so these actions are effectively mouse-only. Please switch them to semantic<button type="button">controls, and use a button/link for the source path.Suggested direction
- <div className="close-button flex-shrink-0 cursor-pointer" onClick={onClose}> + <button + type="button" + className="close-button flex-shrink-0 cursor-pointer" + onClick={onClose} + aria-label="Close script error" + > <IconX size={16} strokeWidth={1.5} /> - </div> + </button> - <span - className="script-error-file-path" - onClick={sourceInfo ? handleNavigate : undefined} - title={sourceInfo ? `Open ${errorContext.filePath}` : undefined} - > - {errorContext.filePath} - </span> + <button + type="button" + className="script-error-file-path" + onClick={handleNavigate} + title={`Open ${errorContext.filePath}`} + > + {errorContext.filePath} + </button> - <div - className="script-error-stack-toggle" - onClick={() => setShowStack(!showStack)} - > + <button + type="button" + className="script-error-stack-toggle" + onClick={() => setShowStack((value) => !value)} + > {showStack ? <IconChevronDown size={14} /> : <IconChevronRight size={14} />} <span>{showStack ? 'Hide' : 'Show'} stack trace</span> - </div> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js` around lines 98 - 129, Replace the non-interactive div/span controls with semantic, keyboard-accessible elements: change the close-button div (which uses onClose) to a <button type="button"> and add an accessible label (aria-label) referencing onClose; change the script-error-file-path span (which uses handleNavigate when sourceInfo exists and shows errorContext.filePath) to a <button type="button"> when it navigates (or an <a> if it performs navigation) and preserve title and disabled/non-interactive state when sourceInfo is absent; change the script-error-stack-toggle div (which flips setShowStack and uses showStack) to a <button type="button">, add aria-expanded={showStack} and an appropriate aria-controls if you render the stack content elsewhere; retain existing classes and handlers but move onClick to the element’s onClick prop and ensure keyboard activation works.
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/network/index.js (1)
447-523: Add JSDoc forbuildErrorContext().This helper now owns most of the error-to-source mapping contract, especially the expected
scriptMetadatashape. A short JSDoc block here would make the new flow much easier to maintain.As per coding guidelines, "Add JSDoc comments to abstractions for additional details".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/index.js` around lines 447 - 523, Add a JSDoc block above buildErrorContext describing its purpose (map runtime error -> source context), parameters (error, scriptType, itemPathname, collectionPath, scriptMetadata) and the expected shape of scriptMetadata (fields used by adjustLineNumber, resolveSegmentError, adjustStackTrace, etc.), the return value (object with errorType, filePath, errorLine, lines, stack or null), and note side-effects/assumptions (file extensions checked: .bru/.yml, use of cache Map). Reference the related helpers used inside (parseErrorLocation, adjustLineNumber, resolveSegmentError, getSourceContext, getErrorTypeName, adjustStackTrace, findScriptBlockStartLine, findYmlScriptBlockStartLine, findScriptBlockEndLine, findYmlScriptBlockEndLine) so maintainers know dependencies and which metadata fields are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-electron/src/ipc/network/index.js`:
- Around line 496-512: The filter is using blockEndLine as if it's a 1-indexed
absolute line but findScriptBlockEndLine (for .bru) returns a 0-index/relative
value; normalize the .bru end-line before comparison by computing a
normalizedBlockEndLine (e.g., if isBru and blockEndLine exists, add 1 to convert
to 1-index/absolute) and use normalizedBlockEndLine in the lines.filter boundary
check instead of raw blockEndLine; update references around blockEndLine,
blockOffset, and the filter that uses l.lineNumber so the last line of a .bru
script block is included correctly.
---
Duplicate comments:
In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js`:
- Around line 98-129: Replace the non-interactive div/span controls with
semantic, keyboard-accessible elements: change the close-button div (which uses
onClose) to a <button type="button"> and add an accessible label (aria-label)
referencing onClose; change the script-error-file-path span (which uses
handleNavigate when sourceInfo exists and shows errorContext.filePath) to a
<button type="button"> when it navigates (or an <a> if it performs navigation)
and preserve title and disabled/non-interactive state when sourceInfo is absent;
change the script-error-stack-toggle div (which flips setShowStack and uses
showStack) to a <button type="button">, add aria-expanded={showStack} and an
appropriate aria-controls if you render the stack content elsewhere; retain
existing classes and handlers but move onClick to the element’s onClick prop and
ensure keyboard activation works.
---
Nitpick comments:
In `@packages/bruno-electron/src/ipc/network/index.js`:
- Around line 447-523: Add a JSDoc block above buildErrorContext describing its
purpose (map runtime error -> source context), parameters (error, scriptType,
itemPathname, collectionPath, scriptMetadata) and the expected shape of
scriptMetadata (fields used by adjustLineNumber, resolveSegmentError,
adjustStackTrace, etc.), the return value (object with errorType, filePath,
errorLine, lines, stack or null), and note side-effects/assumptions (file
extensions checked: .bru/.yml, use of cache Map). Reference the related helpers
used inside (parseErrorLocation, adjustLineNumber, resolveSegmentError,
getSourceContext, getErrorTypeName, adjustStackTrace, findScriptBlockStartLine,
findYmlScriptBlockStartLine, findScriptBlockEndLine, findYmlScriptBlockEndLine)
so maintainers know dependencies and which metadata fields are required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08678ff6-e617-485c-8f64-a021a808e00d
📒 Files selected for processing (6)
packages/bruno-app/src/components/CodeSnippet/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/ScriptError/index.jspackages/bruno-app/src/utils/source-context.jspackages/bruno-app/src/utils/source-context.spec.jspackages/bruno-electron/src/ipc/network/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/components/CodeSnippet/StyledWrapper.js
- packages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ResponsePane/ScriptError/index.js (1)
67-93: Consider adding a guard forcollectionbefore accessing its properties.
handleNavigateaccessescollection.uidacross multiple branches without verifyingcollectionexists. Whilecollectionis a required prop, a defensive check would prevent runtime errors if the prop is accidentally omitted.🛡️ Proposed guard
const handleNavigate = () => { - if (!sourceInfo) return; + if (!sourceInfo || !collection) return; const settingsTab = scriptPhase === 'test' ? 'tests' : 'script';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js` around lines 67 - 93, handleNavigate currently assumes collection is defined and reads collection.uid in several branches which can throw if the prop is missing; add a defensive guard at the start of handleNavigate (e.g., if (!sourceInfo || !collection) return) or specifically ensure collection exists before using collection.uid, and update all branches that call dispatch(addTab(...)), dispatch(updateSettingsSelectedTab(...)), dispatch(updatedFolderSettingsSelectedTab(...)) and any updateScriptPaneTab calls to only reference collection.uid when collection is truthy so runtime errors are avoided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/FolderSettings/Script/index.js`:
- Around line 23-38: The component reads script state from the wrong tab:
replace the lookup that sets focusedTab to use folder.uid instead of
activeTabUid so reads and writes target the same entity; update the code that
computes focusedTab (the find over tabs) to use folder.uid, ensuring
scriptPaneTab, activeTab (from getDefaultTab) and setActiveTab (which dispatches
updateScriptPaneTab with folder.uid) are all consistent.
---
Nitpick comments:
In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js`:
- Around line 67-93: handleNavigate currently assumes collection is defined and
reads collection.uid in several branches which can throw if the prop is missing;
add a defensive guard at the start of handleNavigate (e.g., if (!sourceInfo ||
!collection) return) or specifically ensure collection exists before using
collection.uid, and update all branches that call dispatch(addTab(...)),
dispatch(updateSettingsSelectedTab(...)),
dispatch(updatedFolderSettingsSelectedTab(...)) and any updateScriptPaneTab
calls to only reference collection.uid when collection is truthy so runtime
errors are avoided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bcd6089f-be5b-459b-b3e0-fae1d784179b
📒 Files selected for processing (3)
packages/bruno-app/src/components/CollectionSettings/Script/index.jspackages/bruno-app/src/components/FolderSettings/Script/index.jspackages/bruno-app/src/components/ResponsePane/ScriptError/index.js
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ResponsePane/ScriptError/index.js (1)
179-214: Clarify: Closing any error card dismisses all errors.All
ScriptErrorCardcomponents receive the sameonClosecallback, so clicking close on any card will hide all error cards. If this is intentional (batch dismiss), consider adding a comment. If individual dismissal is desired, you'd need per-error state management.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js` around lines 179 - 214, The three ScriptErrorCard instances all share the same onClose prop so closing any card clears all errors; update the component to provide per-card close handlers (e.g., onClosePreRequest, onClosePostResponse, onCloseTest) instead of the single onClose, and have each handler only clear its corresponding error state/prop (preRequestError, postResponseError, testScriptError) or call the existing close function with a scriptPhase identifier ("pre-request", "post-response", "test") so only that card is dismissed; modify the JSX to pass these per-card handlers into the ScriptErrorCard onClose prop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js`:
- Around line 179-214: The three ScriptErrorCard instances all share the same
onClose prop so closing any card clears all errors; update the component to
provide per-card close handlers (e.g., onClosePreRequest, onClosePostResponse,
onCloseTest) instead of the single onClose, and have each handler only clear its
corresponding error state/prop (preRequestError, postResponseError,
testScriptError) or call the existing close function with a scriptPhase
identifier ("pre-request", "post-response", "test") so only that card is
dismissed; modify the JSX to pass these per-card handlers into the
ScriptErrorCard onClose prop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05c642cd-f466-4d51-b803-0b844b039986
📒 Files selected for processing (5)
packages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/ScriptError/index.jspackages/bruno-app/src/components/ResponsePane/ScriptErrorIcon/index.jspackages/bruno-app/src/components/ResponsePane/index.jspackages/bruno-app/src/components/RunnerResults/ResponsePane/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/ResponsePane/index.js
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/bruno-js/src/utils/error-formatter.js (1)
67-96:⚠️ Potential issue | 🟠 MajorTrack brace nesting before closing the
.bruscript block.Line 88 still treats the first column-0
}as the end of the Bruno block. Any top-levelif/for/tryinside the script closes at column 0 too, sopackages/bruno-electron/src/ipc/network/build-error-context.jswill trim the snippet at that nested brace and hide the remaining script. Please deriveresultfrom brace depth and only stop when the outerscript:* {}closes.packages/bruno-app/src/components/ResponsePane/ScriptError/index.js (1)
33-37:⚠️ Potential issue | 🟡 MinorUse cross-platform path handling instead of hardcoded forward slashes.
Lines 33-37 use hardcoded forward slash operations (
startsWith,slice,replace(/^\//)) which may fail on Windows wherenode.pathnamecould contain backslashes. Consider usingpathutilities or the existinggetRelativePathhelper fromutils/common/path.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js` around lines 33 - 37, The folderRelPath calculation uses string ops on node.pathname and collectionPathname which assume POSIX slashes; replace this with a cross-platform path helper (e.g., import and call getRelativePath from utils/common/path.js) to compute the relative path between collectionPathname and node.pathname, then join that result with folderFileName using path-safe join logic so node.pathname, collectionPathname and folderFileName are handled correctly across platforms; update references to node.pathname, collectionPathname, folderRelPath and folderFileName accordingly.
🧹 Nitpick comments (3)
packages/bruno-electron/src/utils/collection.js (1)
214-215: UseDEFAULT_COLLECTION_FORMAThere instead of duplicating'bru'.This is a second source of truth for the default collection format. Reusing the imported constant keeps this mapping aligned if the filestore default changes later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/utils/collection.js` around lines 214 - 215, Replace the hard-coded default 'bru' with the shared constant: change the line using collection.format fallback to use DEFAULT_COLLECTION_FORMAT (i.e. const format = collection.format || DEFAULT_COLLECTION_FORMAT) and leave the subsequent const config = FORMAT_CONFIG[format]; ensure DEFAULT_COLLECTION_FORMAT is imported into this module so the fallback stays in sync with the filestore default.tests/script-errors/script-errors.spec.ts (1)
56-59: Redundant locator re-assignment.Lines 57-58 re-assign
seandlocatorswhich were already initialized inbeforeAll(lines 49-50). This duplication is unnecessary since both use the samepageWithUserDatapage fixture.♻️ Proposed removal
test('1. Pre-request ReferenceError shows error card with correct details', async ({ pageWithUserData: page }) => { - se = buildScriptErrorLocators(page); - locators = buildCommonLocators(page); - await test.step('Open request and send', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/script-errors/script-errors.spec.ts` around lines 56 - 59, Remove the redundant re-assignment of the locator variables in the test: delete the lines that set se = buildScriptErrorLocators(page) and locators = buildCommonLocators(page) inside the '1. Pre-request ReferenceError...' test since se and locators are already initialized in beforeAll using the same page fixture; keep the test using the existing se and locators variables (functions referenced: buildScriptErrorLocators, buildCommonLocators, variables: se, locators, beforeAll).packages/bruno-electron/src/ipc/network/build-error-context.js (1)
88-90: Consider logging suppressed errors for debugging.The empty
catchblock silently swallows all exceptions. While returningnullis correct behavior, logging the error (at debug level) would aid troubleshooting when error context unexpectedly fails to build.♻️ Proposed enhancement
- } catch { + } catch (err) { + // Log at debug level to aid troubleshooting without polluting user-facing output + console.debug('[buildErrorContext] Failed to build error context:', err?.message || err); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/build-error-context.js` around lines 88 - 90, The empty catch in buildErrorContext (the try/catch that currently does "catch { return null; }") should log the caught exception at debug level before returning null; modify the catch to accept the error (e.g., "catch (err)"), call the module's logger.debug or console.debug with a concise message and the error (e.g., "Failed to build error context:" and err) and then return null so behavior is unchanged while preserving useful debug information.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/CollectionSettings/Script/index.js`:
- Around line 27-33: The activeTab is being recalculated on every render via
getDefaultTab() (which checks requestScript) when scriptPaneTab is falsy,
causing the pane to flip mid-edit; change logic so the default is computed once
and then persisted to Redux: on mount, if scriptPaneTab is null/undefined call
getDefaultTab() and dispatch an action to set scriptPaneTab to that value (use
the same getDefaultTab, requestScript, and scriptPaneTab identifiers), then
derive activeTab exclusively from scriptPaneTab thereafter so UI no longer
toggles while editing.
In `@packages/bruno-app/src/components/FolderSettings/Script/index.js`:
- Around line 27-33: The computed default tab (getDefaultTab) is being
recalculated every render while scriptPaneTab is nullish, causing the tab to
flip when requestScript changes; capture the initial default once and use that
stored value as the fallback instead of recomputing. Concretely, create a stable
default holder (e.g., useRef or useState with a lazy initializer) populated by
getDefaultTab() on first render, then compute activeTab as scriptPaneTab ??
storedDefaultTab (referencing getDefaultTab, scriptPaneTab, requestScript, and
activeTab) so the fallback remains stable while the controlled value is missing.
In `@packages/bruno-electron/src/utils/collection.js`:
- Around line 177-189: The loop currently builds segment ranges from wrapped[i]
and pushes them into a wrapper-level segments array, causing off-by-one mapping
when resolveSegmentError reads metadata.segments while adjustLineNumber expects
wrapper-starts; change the logic in the for-loop that builds segments so that
when you create a segment entry (using startLine/endLine and segmentSources[i])
you attach those segments to the corresponding script body metadata (e.g., set
metadata on scripts[i] or assign scriptsMetadata[i].segments) rather than to the
wrapper-level collection; keep requestStartLine/requestEndLine assignment for
the wrapper (metadata.requestStartLine/requestEndLine) but ensure the segments
array stored on the script body matches the unwrapped source ranges so
resolveSegmentError uses correct startLine/endLine (reference
variables/functions: wrapped, scripts, metadata, segments, requestIndex,
segmentSources, adjustLineNumber(), resolveSegmentError()).
In `@tests/script-errors/script-errors.spec.ts`:
- Around line 43-54: Rename the test fixtures folder from "collections" to
"collection" and update all references in this test and related files to point
to the new path; specifically update any calls that load fixtures for these
tests (e.g., where setSandboxMode is invoked with 'script-errors-test' and
'collection-script-error') and any locator/build helpers such as
buildScriptErrorLocators or buildCommonLocators that assume the old fixtures
path so they import from tests/script-errors/fixtures/collection instead of
tests/script-errors/fixtures/collections.
---
Duplicate comments:
In `@packages/bruno-app/src/components/ResponsePane/ScriptError/index.js`:
- Around line 33-37: The folderRelPath calculation uses string ops on
node.pathname and collectionPathname which assume POSIX slashes; replace this
with a cross-platform path helper (e.g., import and call getRelativePath from
utils/common/path.js) to compute the relative path between collectionPathname
and node.pathname, then join that result with folderFileName using path-safe
join logic so node.pathname, collectionPathname and folderFileName are handled
correctly across platforms; update references to node.pathname,
collectionPathname, folderRelPath and folderFileName accordingly.
---
Nitpick comments:
In `@packages/bruno-electron/src/ipc/network/build-error-context.js`:
- Around line 88-90: The empty catch in buildErrorContext (the try/catch that
currently does "catch { return null; }") should log the caught exception at
debug level before returning null; modify the catch to accept the error (e.g.,
"catch (err)"), call the module's logger.debug or console.debug with a concise
message and the error (e.g., "Failed to build error context:" and err) and then
return null so behavior is unchanged while preserving useful debug information.
In `@packages/bruno-electron/src/utils/collection.js`:
- Around line 214-215: Replace the hard-coded default 'bru' with the shared
constant: change the line using collection.format fallback to use
DEFAULT_COLLECTION_FORMAT (i.e. const format = collection.format ||
DEFAULT_COLLECTION_FORMAT) and leave the subsequent const config =
FORMAT_CONFIG[format]; ensure DEFAULT_COLLECTION_FORMAT is imported into this
module so the fallback stays in sync with the filestore default.
In `@tests/script-errors/script-errors.spec.ts`:
- Around line 56-59: Remove the redundant re-assignment of the locator variables
in the test: delete the lines that set se = buildScriptErrorLocators(page) and
locators = buildCommonLocators(page) inside the '1. Pre-request
ReferenceError...' test since se and locators are already initialized in
beforeAll using the same page fixture; keep the test using the existing se and
locators variables (functions referenced: buildScriptErrorLocators,
buildCommonLocators, variables: se, locators, beforeAll).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba13b930-c94c-4d9b-853c-43fe5eb8628c
📒 Files selected for processing (28)
packages/bruno-app/src/components/CodeSnippet/index.jspackages/bruno-app/src/components/CollectionSettings/Script/index.jspackages/bruno-app/src/components/FolderSettings/Script/index.jspackages/bruno-app/src/components/ResponsePane/ScriptError/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/ScriptError/index.jspackages/bruno-app/src/components/ResponsePane/ScriptErrorIcon/index.jspackages/bruno-app/src/components/ResponsePane/index.jspackages/bruno-app/src/components/RunnerResults/ResponsePane/index.jspackages/bruno-converters/tests/postman/postman-translations/postman-comments.spec.jspackages/bruno-electron/src/ipc/network/build-error-context.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/utils/collection.jspackages/bruno-js/src/index.jspackages/bruno-js/src/utils/error-formatter.jstests/script-errors/fixtures/collections/collection-script-error/bruno.jsontests/script-errors/fixtures/collections/collection-script-error/collection.brutests/script-errors/fixtures/collections/collection-script-error/simple-request.brutests/script-errors/fixtures/collections/script-errors-test/bruno.jsontests/script-errors/fixtures/collections/script-errors-test/collection.brutests/script-errors/fixtures/collections/script-errors-test/error-subfolder/folder-request.brutests/script-errors/fixtures/collections/script-errors-test/error-subfolder/folder.brutests/script-errors/fixtures/collections/script-errors-test/multiple-errors.brutests/script-errors/fixtures/collections/script-errors-test/post-response-type-error.brutests/script-errors/fixtures/collections/script-errors-test/pre-request-ref-error.brutests/script-errors/fixtures/collections/script-errors-test/test-script-error.brutests/script-errors/init-user-data/preferences.jsontests/script-errors/script-errors.spec.tstests/utils/page/locators.ts
✅ Files skipped from review due to trivial changes (4)
- tests/script-errors/fixtures/collections/script-errors-test/collection.bru
- tests/script-errors/fixtures/collections/script-errors-test/bruno.json
- tests/script-errors/init-user-data/preferences.json
- tests/script-errors/fixtures/collections/collection-script-error/bruno.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bruno-app/src/components/RunnerResults/ResponsePane/index.js
- packages/bruno-electron/src/ipc/network/index.js
- packages/bruno-app/src/components/ResponsePane/index.js
…igation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 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.
… handling in prepare-request
- 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.
- 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.
…consolidate error formatter imports
…ion and folder UIDthe respective collection and folder UID instead of the activeTabUid.
…twork 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.
- 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.
…riptError component
…anding of error handling
- 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.
…ErrorContext - 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.
- 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.
…-context utilities
…lity and maintainability
sanish-bruno
force-pushed
the
feat/script-error-enhancement
branch
from
5a05d18 to
7aa0a92
Compare
bijin-bruno
reviewed
Mar 17, 2026
| const focusedTab = find(tabs, (t) => t.uid === collection.uid); | ||
| const scriptPaneTab = focusedTab?.scriptPaneTab; | ||
|
|
||
| // Default to post-response if pre-request script is empty (only when scriptPaneTab is null/undefined) |
Collaborator
There was a problem hiding this comment.
Comment in bracket is irrelevant here. Move it to the relevant line of code
| const normalizedPath = normalizePath(filePath); | ||
|
|
||
| const isFolderFile = /(?:^|\/)folder\.(?:bru|ya?ml)$/.test(normalizedPath); | ||
| const isCollectionFile = normalizedPath === 'collection.bru' || /^opencollection\.ya?ml$/.test(normalizedPath); |
Collaborator
There was a problem hiding this comment.
yaml not supported. cleanup for readability
| const tests = translatedScripts.get(item.uid).request?.script?.res; | ||
| const translated = translatedScripts.get(item.uid); | ||
| const script = translated.request?.script?.req; | ||
| const tests = translated.request?.script?.res; |
Collaborator
There was a problem hiding this comment.
Could've avoided this change
| return { data, dataBuffer }; | ||
| }; | ||
|
|
||
| const posixifyPath = (p) => (p ? p.replace(/\\/g, '/') : p); |
Collaborator
There was a problem hiding this comment.
This is already available in packages/bruno-electron/src/utils/filesystem.js
Please take the latest from main and update accordingly.
|
|
||
| /** Find the 1-indexed last content line of a script block in a .yml file */ | ||
| const findYmlScriptBlockEndLine = (filePath, scriptType, cache = null) => { | ||
| if (!filePath.endsWith('.yml') && !filePath.endsWith('.yaml')) return null; |
| @@ -0,0 +1 @@ | |||
| { "version": "1", "name": "collection-script-error", "type": "collection", "ignore": ["node_modules", ".git"] } | |||
| @@ -0,0 +1 @@ | |||
| { "version": "1", "name": "script-errors-test", "type": "collection", "ignore": ["node_modules", ".git"] } | |||
| */ | ||
| const sendAndWaitForErrorCard = async (page: Page) => { | ||
| const { request } = buildCommonLocators(page); | ||
| const se = buildScriptErrorLocators(page); |
…roved testability
…ntextV2 - 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.
…oved 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.
bijin-bruno
approved these changes
Mar 20, 2026
6 tasks
chirag-bruno
pushed a commit
to chirag-bruno/bruno
that referenced
this pull request
Mar 24, 2026
…ing of untranslated pm commands (usebruno#7449) * feat: enhance ScriptError with source context, code snippets, and navigation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: remove auto-commenting of untranslated pm commands during import Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com>
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #2467 partially
Jira
Screen.Recording.2026-03-16.at.6.35.32.PM.mov
This PR introduces two improvements:
1. Enhanced ScriptError display with source context and navigation
When a pre-request, post-response, or test script throws an error, the ScriptError card now shows:
TypeError,ReferenceError) and the relative path to the file where the error occurredThe error context is computed in the Electron main process (
buildErrorContext) by:New files:
CodeSnippetcomponent — reusable code display with line numbers and error highlightingsource-context.jsutility — groups and formats error context for displayerror-formatter.js— error location parsing, line adjustment, and stack trace formattingcollection.jsutilities — functions to find script block start/end lines in.bruand.ymlfiles2. Remove auto-commenting of untranslated pm commands during import
Previously, when importing a Postman collection, any
pm.*API calls that couldn't be translated to Bruno equivalents were commented out with//prefixes. This was confusing because users couldn't tell which lines were original comments vs auto-disabled code.Now untranslated
pm.*calls are left as-is in the imported script. They will throw clear runtime errors if executed, which is more transparent than silently commenting them out.Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests