Skip to content

#bsq -- fix: handle missing location header in redirect error response#7725

Open
Pragadesh-45 wants to merge 1 commit intousebruno:mainfrom
Pragadesh-45:fix/display-timeline-for-302-with-no-location-header
Open

#bsq -- fix: handle missing location header in redirect error response#7725
Pragadesh-45 wants to merge 1 commit intousebruno:mainfrom
Pragadesh-45:fix/display-timeline-for-302-with-no-location-header

Conversation

@Pragadesh-45
Copy link
Copy Markdown
Contributor

@Pragadesh-45 Pragadesh-45 commented Apr 10, 2026

fixes: #5880

Description

  • Added a check for the presence of the location header in the error response during redirect handling. If the header is absent, the error response is augmented with the timeline and the promise is rejected accordingly.

Contribution Checklist:

  • I've used AI significantly to create this pull request
  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Enhanced error handling for HTTP redirect responses that are missing required headers, ensuring more reliable and graceful handling of network requests in edge cases.

- Added a check for the presence of the location header in the error response during redirect handling. If the header is absent, the error response is augmented with the timeline and the promise is rejected accordingly.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

Walkthrough

A guard was added to the redirect-handling logic in the axios instance to prevent processing invalid redirects. When a redirect response lacks a Location header, the error is now immediately rejected with timeline context attached, preventing undefined URL errors.

Changes

Cohort / File(s) Summary
Redirect validation guard
packages/bruno-electron/src/ipc/network/axios-instance.js
Added early-return validation for redirect responses missing the Location header, attaching timeline context before rejecting the error.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • PR #7426 — Modifies redirect-handling behavior in the same file, addressing non-localhost redirect DNS lookup concerns.

Suggested labels

size/L

Suggested reviewers

  • helloanoop
  • bijin-bruno
  • naman-bruno

Poem

🔗 A redirect without a location's call,
Led to errors that vexed one and all,
Now guards stand tall, catching the flaw,
With timeline in hand—no more "url" saw! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR adds a guard for missing Location headers in redirect responses, which addresses the root cause of the reported URL error by preventing undefined redirect URLs.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to redirect error handling with missing location headers, directly addressing the linked issue #5880.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title directly and specifically describes the main change: adding handling for a missing Location header in redirect error responses, which aligns with the PR's primary objective and code modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/network/axios-instance.js (1)

330-333: Add explicit error context when rejecting missing-Location redirects.

The guard is correct and prevents the undefined URL path, but it currently rejects without recording why redirect handling stopped. Add a timeline error entry here for clearer diagnostics.

Suggested patch
           if (!locationHeader) {
+            timeline.push({
+              timestamp: new Date(),
+              type: 'error',
+              message: 'Redirect response missing Location header; aborting redirect handling'
+            });
             error.response.timeline = timeline;
             return Promise.reject(error);
           }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bruno-electron/src/ipc/network/axios-instance.js` around lines 330 -
333, When the redirect guard sees a missing locationHeader and rejects, add an
explicit timeline entry to error.response.timeline describing why redirect
handling stopped; for example, before the Promise.reject(error) call push a
descriptive entry (e.g., "redirect_missing_location" with a message and any
useful context like response.status or headers) into error.response.timeline so
diagnostics record the reason; update the block that references locationHeader,
error.response.timeline, and Promise.reject(error) to insert that timeline entry
prior to rejecting.
🤖 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-electron/src/ipc/network/axios-instance.js`:
- Around line 330-333: When the redirect guard sees a missing locationHeader and
rejects, add an explicit timeline entry to error.response.timeline describing
why redirect handling stopped; for example, before the Promise.reject(error)
call push a descriptive entry (e.g., "redirect_missing_location" with a message
and any useful context like response.status or headers) into
error.response.timeline so diagnostics record the reason; update the block that
references locationHeader, error.response.timeline, and Promise.reject(error) to
insert that timeline entry prior to rejecting.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 70b8a60e-3220-4b2f-a55d-9f6a7c7858fa

📥 Commits

Reviewing files that changed from the base of the PR and between cd06f28 and 7d6371a.

📒 Files selected for processing (1)
  • packages/bruno-electron/src/ipc/network/axios-instance.js

@Pragadesh-45 Pragadesh-45 changed the title fix: handle missing location header in redirect error response #bsq -- fix: handle missing location header in redirect error response Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strange error message: url argument is not a string or URL.

1 participant