fix: fixes error forwarding to connectors by roroghost17 · Pull Request #5491 · maximhq/bifrost

roroghost17 · 2026-07-23T10:54:13Z

Summary

Tracing spans for failed streaming requests were silently reporting unknown status because error attributes were never stamped when a stream errored out. This fixes two code paths where a BifrostError exists but PopulateLLMResponseAttributes was never called: failed stream requests in the retry loop and streams that fail before the first chunk arrives during deferred span completion.

Changes

In executeRequestWithRetries, added an else if branch to call PopulateLLMResponseAttributes with a nil response and the error when a stream request fails — previously the cast to a non-stream response type would miss this case entirely.
In completeDeferredSpan, added an else if branch to stamp error attributes when a stream errors before producing any chunks, ensuring the span is not left with unknown/empty attributes.

Type of change

Affected areas

How to test

Trigger a streaming request that is expected to fail (e.g., invalid model name, bad credentials, or a provider that returns an error before streaming begins) and verify that the resulting trace span contains the correct error attributes rather than reporting unknown status.

go test ./...

Screenshots/Recordings

N/A

Breaking changes

Yes
No

Related issues

N/A

Security considerations

No security implications. This change only affects observability/tracing attribute population.

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

CLAassistant · 2026-07-23T10:54:22Z

All committers have signed the CLA.

coderabbitai · 2026-07-23T10:54:23Z

📝 Walkthrough

Walkthrough

Tracing now populates LLM response attributes for request and streaming failures that produce no response object.

Changes

Tracing error attribute coverage

Layer / File(s)	Summary
Error-path span enrichment `core/bifrost.go`, `core/providers/utils/utils.go`	Request failures after response type assertion and streaming failures without accumulated chunks now pass the error with a nil response to tracing attribute population.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

maximhq/bifrost#5467: Updates related deferred-span tracing instrumentation in completeDeferredSpan.

Suggested reviewers: pratham-mishra04, akshaydeo, tejasghatte

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions error forwarding to connectors, but the PR actually fixes missing tracing attributes for failed streaming requests.	Rename it to describe the tracing fix, e.g. "fix: populate error attributes for failed streaming spans".

✅ Passed checks (4 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.
Description check	✅ Passed	The PR description matches the template well and includes summary, changes, type, affected areas, testing, breaking changes, and security notes.

✨ 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-23-fix_fixes_error_forwarding_to_connectors

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

roroghost17 · 2026-07-23T10:54:28Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

greptile-apps

This repository has used its 100 free open-source review credits for this billing period. An organization admin can enable additional billed reviews, or reviews resume next period.

akshaydeo · 2026-07-23T16:00:14Z

Merge activity

Jul 23, 4:00 PM UTC: A user started a stack merge that includes this pull request via Graphite.
Jul 23, 4:01 PM UTC: Graphite couldn't merge this PR because it was not satisfying all requirements (PR does not have required approvals).
Jul 23, 4:02 PM UTC: A user started a stack merge that includes this pull request via Graphite.
Jul 23, 4:03 PM UTC: @akshaydeo merged this pull request with Graphite.

The base branch was changed.

## Summary Tracing spans for failed streaming requests were silently reporting unknown status because error attributes were never stamped when a stream errored out. This fixes two code paths where a `BifrostError` exists but `PopulateLLMResponseAttributes` was never called: failed stream requests in the retry loop and streams that fail before the first chunk arrives during deferred span completion. ## Changes - In `executeRequestWithRetries`, added an `else if` branch to call `PopulateLLMResponseAttributes` with a nil response and the error when a stream request fails — previously the cast to a non-stream response type would miss this case entirely. - In `completeDeferredSpan`, added an `else if` branch to stamp error attributes when a stream errors before producing any chunks, ensuring the span is not left with unknown/empty attributes. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Trigger a streaming request that is expected to fail (e.g., invalid model name, bad credentials, or a provider that returns an error before streaming begins) and verify that the resulting trace span contains the correct error attributes rather than reporting unknown status. ```sh go test ./... ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications. This change only affects observability/tracing attribute population. ## 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

roroghost17 added 2 commits July 23, 2026 14:58

fix: fixes whitespace trimming on connectors form fields

d16b3ef

fix: fixes error forwarding to connectors

e36ff50

This was referenced Jul 23, 2026

fix: fixes whitespace trimming on connectors form fields #5484

Merged

chore: update OTEL test harness with error scenarios #5492

Merged

roroghost17 marked this pull request as ready for review July 23, 2026 10:55

coderabbitai Bot requested review from Pratham-Mishra04, TejasGhatte and akshaydeo July 23, 2026 10:56

coderabbitai Bot previously approved these changes Jul 23, 2026

View reviewed changes

greptile-apps Bot reviewed Jul 23, 2026

View reviewed changes

roroghost17 mentioned this pull request Jul 23, 2026

chore: ui descriptions cleanup #5493

Merged

18 tasks

akshaydeo previously approved these changes Jul 23, 2026

View reviewed changes

akshaydeo changed the base branch from 07-23-fix_fixes_whitespace_trimming_on_connectors_form_fields to graphite-base/5491 July 23, 2026 16:00

akshaydeo changed the base branch from graphite-base/5491 to dev July 23, 2026 16:00

akshaydeo merged commit 3ae13ae into dev Jul 23, 2026
8 of 9 checks passed

akshaydeo deleted the 07-23-fix_fixes_error_forwarding_to_connectors branch July 23, 2026 16:03

This was referenced Jul 25, 2026

[Bug]: *StreamRequest returns (nil, nil) for empty streams — consumers hang forever on a nil-channel receive #5555

Closed

fix: return closed non-nil channel for empty streams instead of (nil, nil) #5556

Merged

Navigation Menu

Uh oh!

fix: fixes error forwarding to connectors - #5491

fix: fixes error forwarding to connectors#5491
akshaydeo merged 2 commits into
devfrom
07-23-fix_fixes_error_forwarding_to_connectors

roroghost17 commented Jul 23, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Jul 23, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

roroghost17 commented Jul 23, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot left a comment

Uh oh!

akshaydeo commented Jul 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

roroghost17 commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of change

Affected areas

How to test

Screenshots/Recordings

Breaking changes

Related issues

Security considerations

Checklist

Uh oh!

CLAassistant commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

roroghost17 commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

akshaydeo commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

roroghost17 commented Jul 23, 2026 •

edited

Loading

CLAassistant commented Jul 23, 2026 •

edited

Loading

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

roroghost17 commented Jul 23, 2026 •

edited

Loading

akshaydeo commented Jul 23, 2026 •

edited

Loading