feat: latency info on errors - #4867
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (28)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds request-latency tracking utilities and propagates measured latency into Bifrost errors across core request handling, provider implementations, and logging fallback behavior. ChangesLatency plumbing and core dispatcher
Provider latency enrichment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
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. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/providers/openai/openai.go (2)
498-540: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMissing
SetProviderRequestLatencycall inHandleOpenAITextCompletionStreaming.Every other streaming handler in this file (
HandleOpenAIChatCompletionStreamingline 1076,HandleOpenAIResponsesStreamingline 1750,HandleOpenAISpeechStreamRequestline 2375,HandleOpenAITranscriptionStreamRequestline 2820,HandleOpenAIImageGenerationStreamingline 3256,HandleOpenAIImageEditStreamRequestline 4619,PassthroughStreamline 7261) callsproviderUtils.SetProviderRequestLatency(ctx, time.Since(startTime))immediately afteractiveClient.Do(req, resp). This function computeslatency := time.Since(startTime)locally and passes it explicitly toEnrichErrorfor the initial request-level errors, but never stamps it intoctx. As a result, the mid-stream error paths later in this function (lines 628, 640 — unchanged, callingEnrichErrorwithout an explicit latency arg) will have no latency inExtraFields.Latencysince the context key was never set.🐛 Proposed fix
startTime := time.Now() // Make the request err := activeClient.Do(req, resp) + providerUtils.SetProviderRequestLatency(ctx, time.Since(startTime)) if err != nil { defer providerUtils.ReleaseStreamingResponse(ctx, resp) latency := time.Since(startTime)🤖 Prompt for 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. In `@core/providers/openai/openai.go` around lines 498 - 540, HandleOpenAITextCompletionStreaming should record request latency in the context the same way the other streaming handlers do. After activeClient.Do(req, resp) succeeds, call providerUtils.SetProviderRequestLatency(ctx, time.Since(startTime)) before any later error handling or stream processing. This ensures the downstream EnrichError paths in HandleOpenAITextCompletionStreaming can pick up latency from ctx and populate ExtraFields.Latency consistently.
2817-2847: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClassify pre-first-byte stream
Do()failures as upstream connection errors
NewBifrostOperationError(...)in these streaming passthrough branches (openai.go:2836, 3272, 4635, 7277; anthropic.go:2897; gemini.go:4238) stopsexecuteRequestWithRetriesbeforemax_retries, so transient connection/DNS/refused failures won’t retry. Switch them toNewBifrostUpstreamConnectionError(...).🤖 Prompt for 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. In `@core/providers/openai/openai.go` around lines 2817 - 2847, Pre-first-byte streaming Do() failures are currently classified as operation errors, which prevents executeRequestWithRetries from retrying transient connection/DNS/refused failures. Update the streaming passthrough error paths that handle client.Do(req, resp) before the first byte arrives to use providerUtils.NewBifrostUpstreamConnectionError(...) instead of providerUtils.NewBifrostOperationError(...). Apply this change in the openai provider’s streaming request handling and the matching streaming branches in anthropic and gemini so the retry logic treats these failures as upstream connection errors.
🧹 Nitpick comments (3)
core/providers/vertex/vertex.go (2)
4557-4563: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEmpty-stream-body error also misses latency, right after the newly-instrumented block.
SetProviderRequestLatencywas just recorded a few lines above (line 4527), but the "provider returned an empty stream body" error a few lines below returnsNewBifrostOperationErrorunwrapped, so it won't carry the latency this PR is adding elsewhere in the same function.♻️ Proposed fix
bodyStream := resp.BodyStream() if bodyStream == nil { providerUtils.ReleaseStreamingResponse(ctx, resp) - return nil, providerUtils.NewBifrostOperationError( - "provider returned an empty stream body", - fmt.Errorf("provider returned an empty stream body")) + return nil, providerUtils.SetErrorLatencyFromContext(ctx, providerUtils.NewBifrostOperationError( + "provider returned an empty stream body", + fmt.Errorf("provider returned an empty stream body"))) }🤖 Prompt for 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. In `@core/providers/vertex/vertex.go` around lines 4557 - 4563, The empty-stream-body path in the streaming response handling is bypassing the newly added latency instrumentation. Update the error return in the bodyStream nil check within the stream-processing logic to include the recorded request latency the same way the other error paths in this function do, using the existing SetProviderRequestLatency data before calling providerUtils.NewBifrostOperationError. Keep the fix localized to the response-stream handling around bodyStream and the latency instrumentation added earlier in the same function.
3300-3355: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
gcsListAllObjectsmissing the same latency wrap as its siblings.Every other GCS helper in this file (
gcsDownloadObject,gcsFileUploadDirect/Resumable,FileList,fileRetrieveByKey,fileDeleteByKey,fileContentByKey) now wraps itsparseGCSAPIErrorreturn withproviderUtils.SetErrorLatencyFromContext(ctx, ...). This function's identicalapiErrbranch (used byBatchResults→batchResultsByKey) still returns the raw parsed error with no latency attached, sincebatchResultsByKeydoesn't post-processlistErreither.🐛 Proposed fix
var apiErr *schemas.BifrostError if statusCode != fasthttp.StatusOK { - apiErr = parseGCSAPIError(resp.Body(), statusCode, "list") + apiErr = providerUtils.SetErrorLatencyFromContext(ctx, parseGCSAPIError(resp.Body(), statusCode, "list")) }🤖 Prompt for 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. In `@core/providers/vertex/vertex.go` around lines 3300 - 3355, The gcsListAllObjects error path is returning a raw parseGCSAPIError result without the standard latency metadata. Update the apiErr branch in gcsListAllObjects to wrap the parsed GCS error with providerUtils.SetErrorLatencyFromContext(ctx, ...) just like the sibling helpers (for example gcsDownloadObject and fileRetrieveByKey), so BatchResults/batchResultsByKey receives the latency-attached BifrostError consistently.core/providers/vertex/cachedcontents.go (1)
175-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: inconsistent latency-wrapping style vs. sibling methods.
CachedContentCreatewraps the error withproviderUtils.SetErrorLatencyFromContext(ctx, ...), while the siblingcachedContentListByKey/cachedContentRetrieveByKey/cachedContentUpdateByKey/cachedContentDeleteByKey(Lines 253, 326, 430, 515) useproviderUtils.SetErrorLatency(err, latency)directly with the already-available locallatencyvariable. Both should be functionally equivalent here sincectxis typed*schemas.BifrostContext, but using the direct locallatencyavoids the extra context-lookup indirection and keeps the file internally consistent.♻️ Suggested consistency fix
- return nil, providerUtils.SetErrorLatencyFromContext(ctx, parseVertexCachedContentError(resp)) + return nil, providerUtils.SetErrorLatency(parseVertexCachedContentError(resp), latency)🤖 Prompt for 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. In `@core/providers/vertex/cachedcontents.go` around lines 175 - 181, CachedContentCreate uses a different error-latency wrapping style than the sibling cachedContentListByKey, cachedContentRetrieveByKey, cachedContentUpdateByKey, and cachedContentDeleteByKey methods. Update the error return path in CachedContentCreate to wrap parseVertexCachedContentError(resp) with providerUtils.SetErrorLatency using the local latency value, matching the other methods and keeping the file consistent.
🤖 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 `@core/providers/anthropic/anthropic.go`:
- Around line 2880-2898: The error handling in PassthroughStream is using the
wrong Bifrost error type for failures before the first byte is received. Update
the err branch in the activeClient.Do path to return
providerUtils.NewBifrostUpstreamConnectionError(schemas.ErrProviderDoRequest,
err) instead of providerUtils.NewBifrostOperationError(...), while keeping the
existing context-cancelled and timeout handling unchanged.
In `@core/providers/gemini/gemini.go`:
- Around line 4221-4238: Treat pre-first-byte failures from activeClient.Do in
the Gemini streaming path as retriable upstream errors instead of terminal
operation errors. In the error handling block around
providerUtils.ReleaseStreamingResponse and the context canceled/timeout checks,
replace the NewBifrostOperationError branch with
NewBifrostUpstreamConnectionError so executeRequestWithRetries can continue
retrying before max_retries is reached. Keep the existing handling in gemini.go
consistent with the other streaming provider implementations.
---
Outside diff comments:
In `@core/providers/openai/openai.go`:
- Around line 498-540: HandleOpenAITextCompletionStreaming should record request
latency in the context the same way the other streaming handlers do. After
activeClient.Do(req, resp) succeeds, call
providerUtils.SetProviderRequestLatency(ctx, time.Since(startTime)) before any
later error handling or stream processing. This ensures the downstream
EnrichError paths in HandleOpenAITextCompletionStreaming can pick up latency
from ctx and populate ExtraFields.Latency consistently.
- Around line 2817-2847: Pre-first-byte streaming Do() failures are currently
classified as operation errors, which prevents executeRequestWithRetries from
retrying transient connection/DNS/refused failures. Update the streaming
passthrough error paths that handle client.Do(req, resp) before the first byte
arrives to use providerUtils.NewBifrostUpstreamConnectionError(...) instead of
providerUtils.NewBifrostOperationError(...). Apply this change in the openai
provider’s streaming request handling and the matching streaming branches in
anthropic and gemini so the retry logic treats these failures as upstream
connection errors.
---
Nitpick comments:
In `@core/providers/vertex/cachedcontents.go`:
- Around line 175-181: CachedContentCreate uses a different error-latency
wrapping style than the sibling cachedContentListByKey,
cachedContentRetrieveByKey, cachedContentUpdateByKey, and
cachedContentDeleteByKey methods. Update the error return path in
CachedContentCreate to wrap parseVertexCachedContentError(resp) with
providerUtils.SetErrorLatency using the local latency value, matching the other
methods and keeping the file consistent.
In `@core/providers/vertex/vertex.go`:
- Around line 4557-4563: The empty-stream-body path in the streaming response
handling is bypassing the newly added latency instrumentation. Update the error
return in the bodyStream nil check within the stream-processing logic to include
the recorded request latency the same way the other error paths in this function
do, using the existing SetProviderRequestLatency data before calling
providerUtils.NewBifrostOperationError. Keep the fix localized to the
response-stream handling around bodyStream and the latency instrumentation added
earlier in the same function.
- Around line 3300-3355: The gcsListAllObjects error path is returning a raw
parseGCSAPIError result without the standard latency metadata. Update the apiErr
branch in gcsListAllObjects to wrap the parsed GCS error with
providerUtils.SetErrorLatencyFromContext(ctx, ...) just like the sibling helpers
(for example gcsDownloadObject and fileRetrieveByKey), so
BatchResults/batchResultsByKey receives the latency-attached BifrostError
consistently.
🪄 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: a1cb1524-eb81-46b3-add4-746dec17f5f3
📒 Files selected for processing (27)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/azure/azure.gocore/providers/azure/realtime.gocore/providers/bedrock/bedrock.gocore/providers/cohere/cohere.gocore/providers/elevenlabs/elevenlabs.gocore/providers/gemini/batch.gocore/providers/gemini/cachedcontents.gocore/providers/gemini/gemini.gocore/providers/huggingface/huggingface.gocore/providers/mistral/mistral.gocore/providers/openai/openai.gocore/providers/openai/realtime.gocore/providers/openrouter/openrouter.gocore/providers/perplexity/perplexity.gocore/providers/replicate/replicate.gocore/providers/replicate/utils.gocore/providers/runware/runware.gocore/providers/runway/runway.gocore/providers/utils/utils.gocore/providers/utils/utils_test.gocore/providers/vertex/cachedcontents.gocore/providers/vertex/vertex.gocore/providers/vllm/vllm.gocore/schemas/bifrost.goplugins/logging/main.go
a3c163c to
7222d7b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
core/providers/bedrock/bedrock.go (1)
541-596: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
makeStreamingRequest's Do() catch-all also uses generic classification instead of the retriable upstream-connection error.Same convention gap as
replicate/utils.go'slistenToReplicateStreamURLandvllm.go'sTranscriptionStream: the final catch-all (lines 576-582) for a pre-first-byte streamingDo()failure builds a plainschemas.ErrProviderDoRequesterror instead ofNewBifrostUpstreamConnectionError, unlike the corrected pattern used in this same PR byhuggingface.goandvertex.go's streaming paths.🤖 Prompt for 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. In `@core/providers/bedrock/bedrock.go` around lines 541 - 596, The final catch-all error path in makeStreamingRequest is classifying pre-first-byte streaming Do() failures as a generic provider request error instead of the retriable upstream-connection error. Update the respErr handling in makeStreamingRequest to use NewBifrostUpstreamConnectionError for the non-timeout, non-cancel, non-DNS/network cases, matching the streaming patterns used in huggingface.go and vertex.go and keeping the existing latency/error wrapping through SetErrorLatency.Source: Learnings
core/providers/vllm/vllm.go (1)
483-504: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSame streaming Do() catch-all classification gap as
replicate/utils.go.The final catch-all at line 503 uses
NewBifrostOperationErrorfor a pre-first-byte streamingDo()failure, instead ofNewBifrostUpstreamConnectionErrorused byhuggingface.go's andvertex.go's equivalent streaming branches in this same PR. Same fix applies here for retry consistency.🤖 Prompt for 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. In `@core/providers/vllm/vllm.go` around lines 483 - 504, The streaming Do() error handling in provider.streamingClient.Do within the vllm provider uses the wrong catch-all classification for pre-first-byte failures. Update the final non-cancel/non-timeout branch to return a NewBifrostUpstreamConnectionError instead of NewBifrostOperationError, matching the equivalent streaming paths in huggingface.go and vertex.go and keeping retry behavior consistent. Keep the existing latency/error wrapping flow intact with providerUtils.SetErrorLatency.Source: Learnings
core/providers/replicate/utils.go (1)
110-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winStreaming Do() catch-all uses generic error, not the retriable upstream-connection classification.
Cancellation/timeout branches look correct, but the final catch-all wraps
errwithNewBifrostOperationErrorinstead ofNewBifrostUpstreamConnectionError. Per repo convention (PR 4662, applied inhuggingface.go'sImageGenerationStream/ImageEditStreamandvertex.go'sPassthroughStreamin this same PR), pre-first-byte streamingDo()failures should be classified as retriable upstream-connection errors soexecuteRequestWithRetrieshonorsmax_retriesconsistently with the non-streaming path.♻️ Suggested alignment with repo convention
- return nil, nil, providerUtils.SetErrorLatency(providerUtils.NewBifrostOperationError(schemas.ErrProviderDoRequest, err), latency) + // Request failed before the first response byte — classify as retriable upstream connection error, + // matching the non-streaming path (see maximhq/bifrost#4496). + return nil, nil, providerUtils.SetErrorLatency(providerUtils.NewBifrostUpstreamConnectionError(schemas.ErrProviderDoRequest, err), latency)Based on learnings: "treat failures that occur in the pre-first-byte phase of the upstream
Do()call asNewBifrostUpstreamConnectionError... Also ensure retries only occur when the operator configuresmax_retries > 0".🤖 Prompt for 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. In `@core/providers/replicate/utils.go` around lines 110 - 132, The catch-all error path in the streaming Do() flow should be classified as a retriable upstream connection failure instead of a generic operation error. In the Replicate streaming helper in utils.go, keep the cancellation and timeout branches as-is, but change the final return that currently uses NewBifrostOperationError to use NewBifrostUpstreamConnectionError so executeRequestWithRetries can apply the same retry behavior as the non-streaming path. Match the repo convention used in huggingface.go and vertex.go for pre-first-byte streaming Do() failures.Source: Learnings
🤖 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.
Nitpick comments:
In `@core/providers/bedrock/bedrock.go`:
- Around line 541-596: The final catch-all error path in makeStreamingRequest is
classifying pre-first-byte streaming Do() failures as a generic provider request
error instead of the retriable upstream-connection error. Update the respErr
handling in makeStreamingRequest to use NewBifrostUpstreamConnectionError for
the non-timeout, non-cancel, non-DNS/network cases, matching the streaming
patterns used in huggingface.go and vertex.go and keeping the existing
latency/error wrapping through SetErrorLatency.
In `@core/providers/replicate/utils.go`:
- Around line 110-132: The catch-all error path in the streaming Do() flow
should be classified as a retriable upstream connection failure instead of a
generic operation error. In the Replicate streaming helper in utils.go, keep the
cancellation and timeout branches as-is, but change the final return that
currently uses NewBifrostOperationError to use NewBifrostUpstreamConnectionError
so executeRequestWithRetries can apply the same retry behavior as the
non-streaming path. Match the repo convention used in huggingface.go and
vertex.go for pre-first-byte streaming Do() failures.
In `@core/providers/vllm/vllm.go`:
- Around line 483-504: The streaming Do() error handling in
provider.streamingClient.Do within the vllm provider uses the wrong catch-all
classification for pre-first-byte failures. Update the final
non-cancel/non-timeout branch to return a NewBifrostUpstreamConnectionError
instead of NewBifrostOperationError, matching the equivalent streaming paths in
huggingface.go and vertex.go and keeping retry behavior consistent. Keep the
existing latency/error wrapping flow intact with providerUtils.SetErrorLatency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eedb5005-6d29-437b-b1fd-769591207ecd
📒 Files selected for processing (27)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/azure/azure.gocore/providers/azure/realtime.gocore/providers/bedrock/bedrock.gocore/providers/cohere/cohere.gocore/providers/elevenlabs/elevenlabs.gocore/providers/gemini/batch.gocore/providers/gemini/cachedcontents.gocore/providers/gemini/gemini.gocore/providers/huggingface/huggingface.gocore/providers/mistral/mistral.gocore/providers/openai/openai.gocore/providers/openai/realtime.gocore/providers/openrouter/openrouter.gocore/providers/perplexity/perplexity.gocore/providers/replicate/replicate.gocore/providers/replicate/utils.gocore/providers/runware/runware.gocore/providers/runway/runway.gocore/providers/utils/utils.gocore/providers/utils/utils_test.gocore/providers/vertex/cachedcontents.gocore/providers/vertex/vertex.gocore/providers/vllm/vllm.gocore/schemas/bifrost.goplugins/logging/main.go
🚧 Files skipped from review as they are similar to previous changes (21)
- core/providers/openai/realtime.go
- core/providers/gemini/batch.go
- core/providers/runware/runware.go
- core/providers/vertex/cachedcontents.go
- core/bifrost.go
- core/providers/azure/realtime.go
- plugins/logging/main.go
- core/schemas/bifrost.go
- core/providers/utils/utils_test.go
- core/providers/replicate/replicate.go
- core/providers/perplexity/perplexity.go
- core/providers/gemini/cachedcontents.go
- core/providers/cohere/cohere.go
- core/providers/openrouter/openrouter.go
- core/providers/anthropic/anthropic.go
- core/providers/elevenlabs/elevenlabs.go
- core/providers/azure/azure.go
- core/providers/mistral/mistral.go
- core/providers/utils/utils.go
- core/providers/gemini/gemini.go
- core/providers/openai/openai.go
7222d7b to
1d1a633
Compare
Merge activity
|
* upstream/dev: feat: adds multiple teams / customers / bus to connectors (maximhq#4875) fix: small latency return fixes (maximhq#4876) Added missing OpenAI responses methods for lifecycle related tasks (maximhq#3125) feat: latency info on errors (maximhq#4867) feat: add `user_name`, `team_ids`, `team_names`, `customer_ids`, `customer_names`, `business_unit_ids`, `business_unit_names` to log list select columns (maximhq#4866) feat: add multi-value attribution cell with plural fallback for logs columns (maximhq#4865)
3 participants
Summary
Provider errors were missing latency information, making it impossible to know how long a request took when it failed. This PR ensures that all error paths — across every provider, for both streaming and non-streaming requests — carry the measured provider request latency in
BifrostError.ExtraFields.Latency. The logging plugin is updated to read this latency from errors so failed requests are logged with accurate timing.Changes
Latency int64field toBifrostErrorExtraFieldsso errors can carry timing data.BifrostContextKeyProviderRequestLatencycontext key to pass measured latency from HTTP helpers to error constructors.providerUtils:SetErrorLatency— stamps atime.Durationdirectly onto aBifrostError.SetProviderRequestLatency— records measured latency on the context afterclient.Doreturns.SetErrorLatencyFromContext— reads the context-stored latency and stamps it onto an error; used on error returns that bypassEnrichError.EnrichErrorto accept an optional variadiclatencyargument; when provided it stamps the error directly, otherwise it falls back to the context-stored value.MakeRequestWithContextandMakeRequestWithContextFollowRedirectsto callSetProviderRequestLatencyafter the request completes, and updated all error returns insidemakeRequestWithDoFuncto carry latency.SetErrorLatency/SetErrorLatencyFromContext/ updatedEnrichErrorcalls across all providers (Anthropic, Azure, Bedrock, Cohere, ElevenLabs, Gemini, HuggingFace, Mistral, OpenAI, OpenRouter, Perplexity, Replicate, Runware, Runway, VLLM, Vertex) on every error return path, including streaming, passthrough, file, batch, container, and model-listing operations.PostLLMHookto readbifrostErr.ExtraFields.Latencywhen a result is absent, so failed requests are logged with the correct latency instead of zero.bifrostErr.ExtraFields = schemas.BifrostErrorExtraFields{...}assignments inbifrost.gowith the newPopulateExtraFieldshelper.EnrichErrorlatency stamping, context-based latency propagation, andSetErrorLatencyFromContextno-op behavior.Type of change
Affected areas
How to test
go test ./core/... ./plugins/...Trigger a request to any provider with an invalid API key or a network error and verify that the returned
BifrostError.ExtraFields.Latencyis non-zero and reflects the actual time spent waiting for the provider. Check that the logging plugin records the same non-zero latency for the failed request.Breaking changes
Related issues
Security considerations
None. No auth, secrets, or PII are involved; only timing metadata is added to error structs.
Checklist
docs/contributing/README.mdand followed the guidelines