Skip to content

fix: pass routing info headers in error/passthrough paths - #5528

Merged
akshaydeo merged 2 commits into
devfrom
07-24-fix_pass_routing_info_headers_in_error_passthrough_paths
Jul 24, 2026
Merged

akshaydeo merged 2 commits into
devfrom
07-24-fix_pass_routing_info_headers_in_error_passthrough_paths

Conversation

@TejasGhatte

@TejasGhatte TejasGhatte commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Error responses and passthrough responses (both streaming and non-streaming) were not emitting x-bifrost-* routing identity headers, making it impossible for callers to determine which provider ran or whether a fallback fired when a request failed or was proxied verbatim.

Changes

  • Added ApplyBifrostErrorResponseHeaders in lib/responseheaders.go to bridge BifrostErrorExtraFields to the shared ApplyBifrostResponseHeaders writer, since error and success paths use different extra-fields types.
  • Called ApplyBifrostErrorResponseHeaders in handlers/utils.go (SendBifrostError) and integrations/utils.go (sendStreamError) so all error responses carry routed-identity headers. Applied after provider-header forwarding so a chained upstream's x-bifrost-* headers cannot overwrite bifrost's own identity.
  • Called ApplyBifrostResponseHeaders in integrations/router.go for both handlePassthroughNonStream and handlePassthroughStream, since passthrough responses forward provider bytes 1:1 and cannot carry extra_fields in the body.
  • When bifrostCtx is nil (native error path), context-only headers such as x-bifrost-fallback-index and x-bifrost-upstream-latency are intentionally omitted; routing_info-* headers still report which provider ran and whether a fallback fired.
  • Added TestApplyBifrostErrorResponseHeaders covering fallback-exhausted errors, the nil-context native error path, and empty extra fields.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./transports/bifrost-http/lib/...
go test ./transports/bifrost-http/integrations/...
go test ./transports/bifrost-http/handlers/...

Send a request that triggers a provider error or exhausts all fallbacks and inspect the response headers — x-bifrost-provider, x-bifrost-routing-info-is-fallback, x-bifrost-routing-info-provider, and related headers should be present. Repeat with a passthrough route (streaming and non-streaming) and confirm the same identity headers appear after provider-forwarded headers.

Screenshots/Recordings

N/A

Breaking changes

  • No

Related issues

N/A

Security considerations

No auth, secrets, or PII implications. Headers added are routing metadata already present on success responses.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Routed-identity headers are now applied to Bifrost error responses and streaming or non-streaming passthrough responses. A shared error-header helper maps error metadata, with tests covering fallback, native, and empty metadata cases.

Changes

Routed identity header propagation

Layer / File(s) Summary
Error response header contract
transports/bifrost-http/lib/responseheaders.go, transports/bifrost-http/lib/responseheaders_test.go
Error metadata is converted into routed-identity response headers, with coverage for fallback-exhausted, native, and empty-field responses.
Response path header wiring
transports/bifrost-http/handlers/utils.go, transports/bifrost-http/integrations/utils.go, transports/bifrost-http/integrations/router.go
Error handlers and streaming/non-streaming passthrough paths apply routed-identity headers after provider headers are forwarded.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • maximhq/bifrost#5485: Modifies the shared routed-identity response-header propagation used by these paths.
  • maximhq/bifrost#5495: Modifies the related routed-identity header handling in streaming HTTP response paths.

Suggested reviewers: roroghost17, akshaydeo, pratham-mishra04

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: adding routing info headers to error and passthrough paths.
Description check ✅ Passed The description follows the required template and includes summary, changes, testing, affected areas, breaking changes, security, and checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-24-fix_pass_routing_info_headers_in_error_passthrough_paths

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

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@TejasGhatte
TejasGhatte marked this pull request as ready for review July 24, 2026 10:50

TejasGhatte commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transports/bifrost-http/integrations/router.go`:
- Line 3384: Update the response-header handling around
ApplyBifrostResponseHeaders so the streaming response always retains
Cache-Control: no-cache after passthrough headers are applied. Reassert the
no-cache directive immediately after the call, or exclude Cache-Control from the
provider headers handled there, while preserving the existing passthrough
behavior for other headers.

In `@transports/bifrost-http/integrations/utils.go`:
- Around line 169-170: Update the error-response header flow around
ApplyBifrostErrorResponseHeaders to clear the complete managed x-bifrost-*
header set after forwarding provider headers, including stale routed identity
fields, then apply bifrostErr.ExtraFields so route-derived values overwrite
forwarded headers. Preserve the existing provider-header forwarding and
route-derived header behavior.
- Around line 169-170: Reserve the managed x-bifrost-* namespace after
provider-header forwarding and before applying routed identity fields. In
transports/bifrost-http/integrations/utils.go lines 169-170, clear every managed
Bifrost identity header before ApplyBifrostErrorResponseHeaders; apply the same
clearing in transports/bifrost-http/integrations/router.go lines 3287-3290 and
3381-3384 for non-stream and stream passthrough paths, respectively. Add a
regression test covering upstream Bifrost identity collisions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cf881b5b-10e7-4200-a4a5-3a64b0853dda

📥 Commits

Reviewing files that changed from the base of the PR and between 820518a and 1a21bc9.

📒 Files selected for processing (5)
  • transports/bifrost-http/handlers/utils.go
  • transports/bifrost-http/integrations/router.go
  • transports/bifrost-http/integrations/utils.go
  • transports/bifrost-http/lib/responseheaders.go
  • transports/bifrost-http/lib/responseheaders_test.go

Comment thread transports/bifrost-http/integrations/router.go Outdated
Comment thread transports/bifrost-http/integrations/utils.go
@TejasGhatte
TejasGhatte force-pushed the 07-24-fix_pass_routing_info_headers_in_error_passthrough_paths branch from 1a21bc9 to 129aeff Compare July 24, 2026 12:06
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 24, 2026
akshaydeo
akshaydeo previously approved these changes Jul 24, 2026

akshaydeo commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 24, 12:46 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 24, 12:48 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 07-24-fix_replicate_harness_test_and_gemini_extra_param to graphite-base/5528 July 24, 2026 12:47
@akshaydeo
akshaydeo changed the base branch from graphite-base/5528 to dev July 24, 2026 12:47
@akshaydeo
akshaydeo dismissed stale reviews from coderabbitai[bot] and themself July 24, 2026 12:47

The base branch was changed.

@akshaydeo
akshaydeo merged commit 0bd2d4f into dev Jul 24, 2026
6 of 7 checks passed
@akshaydeo
akshaydeo deleted the 07-24-fix_pass_routing_info_headers_in_error_passthrough_paths branch July 24, 2026 12:48
akshaydeo pushed a commit that referenced this pull request Jul 24, 2026
## Summary

Error responses and passthrough responses (both streaming and non-streaming) were not emitting `x-bifrost-*` routing identity headers, making it impossible for callers to determine which provider ran or whether a fallback fired when a request failed or was proxied verbatim.

## Changes

- Added `ApplyBifrostErrorResponseHeaders` in `lib/responseheaders.go` to bridge `BifrostErrorExtraFields` to the shared `ApplyBifrostResponseHeaders` writer, since error and success paths use different extra-fields types.
- Called `ApplyBifrostErrorResponseHeaders` in `handlers/utils.go` (`SendBifrostError`) and `integrations/utils.go` (`sendStreamError`) so all error responses carry routed-identity headers. Applied after provider-header forwarding so a chained upstream's `x-bifrost-*` headers cannot overwrite bifrost's own identity.
- Called `ApplyBifrostResponseHeaders` in `integrations/router.go` for both `handlePassthroughNonStream` and `handlePassthroughStream`, since passthrough responses forward provider bytes 1:1 and cannot carry `extra_fields` in the body.
- When `bifrostCtx` is `nil` (native error path), context-only headers such as `x-bifrost-fallback-index` and `x-bifrost-upstream-latency` are intentionally omitted; `routing_info-*` headers still report which provider ran and whether a fallback fired.
- Added `TestApplyBifrostErrorResponseHeaders` covering fallback-exhausted errors, the nil-context native error path, and empty extra fields.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./transports/bifrost-http/lib/...
go test ./transports/bifrost-http/integrations/...
go test ./transports/bifrost-http/handlers/...
```

Send a request that triggers a provider error or exhausts all fallbacks and inspect the response headers — `x-bifrost-provider`, `x-bifrost-routing-info-is-fallback`, `x-bifrost-routing-info-provider`, and related headers should be present. Repeat with a passthrough route (streaming and non-streaming) and confirm the same identity headers appear after provider-forwarded headers.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] No

## Related issues

N/A

## Security considerations

No auth, secrets, or PII implications. Headers added are routing metadata already present on success responses.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
## Summary

Error responses and passthrough responses (both streaming and non-streaming) were not emitting `x-bifrost-*` routing identity headers, making it impossible for callers to determine which provider ran or whether a fallback fired when a request failed or was proxied verbatim.

## Changes

- Added `ApplyBifrostErrorResponseHeaders` in `lib/responseheaders.go` to bridge `BifrostErrorExtraFields` to the shared `ApplyBifrostResponseHeaders` writer, since error and success paths use different extra-fields types.
- Called `ApplyBifrostErrorResponseHeaders` in `handlers/utils.go` (`SendBifrostError`) and `integrations/utils.go` (`sendStreamError`) so all error responses carry routed-identity headers. Applied after provider-header forwarding so a chained upstream's `x-bifrost-*` headers cannot overwrite bifrost's own identity.
- Called `ApplyBifrostResponseHeaders` in `integrations/router.go` for both `handlePassthroughNonStream` and `handlePassthroughStream`, since passthrough responses forward provider bytes 1:1 and cannot carry `extra_fields` in the body.
- When `bifrostCtx` is `nil` (native error path), context-only headers such as `x-bifrost-fallback-index` and `x-bifrost-upstream-latency` are intentionally omitted; `routing_info-*` headers still report which provider ran and whether a fallback fired.
- Added `TestApplyBifrostErrorResponseHeaders` covering fallback-exhausted errors, the nil-context native error path, and empty extra fields.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./transports/bifrost-http/lib/...
go test ./transports/bifrost-http/integrations/...
go test ./transports/bifrost-http/handlers/...
```

Send a request that triggers a provider error or exhausts all fallbacks and inspect the response headers — `x-bifrost-provider`, `x-bifrost-routing-info-is-fallback`, `x-bifrost-routing-info-provider`, and related headers should be present. Repeat with a passthrough route (streaming and non-streaming) and confirm the same identity headers appear after provider-forwarded headers.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] No

## Related issues

N/A

## Security considerations

No auth, secrets, or PII implications. Headers added are routing metadata already present on success responses.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
## Summary

Error responses and passthrough responses (both streaming and non-streaming) were not emitting `x-bifrost-*` routing identity headers, making it impossible for callers to determine which provider ran or whether a fallback fired when a request failed or was proxied verbatim.

## Changes

- Added `ApplyBifrostErrorResponseHeaders` in `lib/responseheaders.go` to bridge `BifrostErrorExtraFields` to the shared `ApplyBifrostResponseHeaders` writer, since error and success paths use different extra-fields types.
- Called `ApplyBifrostErrorResponseHeaders` in `handlers/utils.go` (`SendBifrostError`) and `integrations/utils.go` (`sendStreamError`) so all error responses carry routed-identity headers. Applied after provider-header forwarding so a chained upstream's `x-bifrost-*` headers cannot overwrite bifrost's own identity.
- Called `ApplyBifrostResponseHeaders` in `integrations/router.go` for both `handlePassthroughNonStream` and `handlePassthroughStream`, since passthrough responses forward provider bytes 1:1 and cannot carry `extra_fields` in the body.
- When `bifrostCtx` is `nil` (native error path), context-only headers such as `x-bifrost-fallback-index` and `x-bifrost-upstream-latency` are intentionally omitted; `routing_info-*` headers still report which provider ran and whether a fallback fired.
- Added `TestApplyBifrostErrorResponseHeaders` covering fallback-exhausted errors, the nil-context native error path, and empty extra fields.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./transports/bifrost-http/lib/...
go test ./transports/bifrost-http/integrations/...
go test ./transports/bifrost-http/handlers/...
```

Send a request that triggers a provider error or exhausts all fallbacks and inspect the response headers — `x-bifrost-provider`, `x-bifrost-routing-info-is-fallback`, `x-bifrost-routing-info-provider`, and related headers should be present. Repeat with a passthrough route (streaming and non-streaming) and confirm the same identity headers appear after provider-forwarded headers.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] No

## Related issues

N/A

## Security considerations

No auth, secrets, or PII implications. Headers added are routing metadata already present on success responses.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants