adds missing cancel() to integration router

[akshaydeo]

Summary

Consolidates context cancellation in createHandler into a single defer at the top of the handler lambda, eliminating scattered and inconsistent cancel() / defer cancel() calls throughout the function. A streamingOwnsCancel flag is introduced to transfer cancel ownership to the streaming path, whose producer goroutine outlives the handler lambda and is responsible for calling cancel itself.

Changes

• Replaced all individual cancel() and defer cancel() calls across every early-return branch with a single defer guarded by a streamingOwnsCancel boolean.
• When the request is determined to be streaming, streamingOwnsCancel is set to true before calling handleStreamingRequest, so the deferred cleanup is skipped and ownership is passed to the streaming goroutine.
• All non-streaming and error paths continue to have cancel called correctly via the centralized defer without any change in behavior.

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/...

Verify that streaming responses complete without context cancellation errors, and that non-streaming and error paths do not leak contexts.

Screenshots/Recordings

N/A

Breaking changes

• Yes
• No

Related issues

N/A

Security considerations

No security implications. This change only affects context lifecycle management within the HTTP handler.

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]

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.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 13f91cb8-0fa1-4ad4-bfb8-44d3c486735d

📥 Commits

Reviewing files that changed from the base of the PR and between 4fdb7c7 and e159bcd.

📒 Files selected for processing (1)

• transports/bifrost-http/integrations/router.go

---

📝 Walkthrough

Summary by CodeRabbit

• Refactor
  • Optimized request handler cleanup logic for improved resource management and code maintainability.

Walkthrough

This change consolidates cancellation lifetime management in GenericRouter.createHandler by introducing a streamingOwnsCancel flag and a single centralized deferred cleanup. Redundant per-branch defer cancel() calls are removed throughout request parsing error paths, request type conversions, and handler dispatch logic. For streaming requests, ownership is explicitly transferred to prevent premature cancellation.

Changes

Centralized cancellation cleanup in request handler

Layer / File(s)	Summary
Centralized cancellation infrastructure transports/bifrost-http/integrations/router.go	Introduce streamingOwnsCancel flag and single deferred cleanup that calls cancel() only when streaming does not take ownership. Async-retrieve fast-path setup moves after centralized integration-type context assignment.
Request parsing and validation error paths transports/bifrost-http/integrations/router.go	Large-payload detection, custom request parser, and JSON unmarshal error paths no longer explicitly call cancel(). All rely on centralized defer cleanup for resource release.
Short-circuit handlers and model extraction transports/bifrost-http/integrations/router.go	Short-circuit handler and GetRequestModel error handling remove per-branch cancellation defers. Both depend on centralized cleanup unless streaming takes ownership later.
Request type conversions and handler dispatch transports/bifrost-http/integrations/router.go	Batch, file, container, container-file, cached-content, and inference request conversions remove their per-branch defer cancel() calls while preserving conversion logic and dispatch to dedicated handlers with existing error handling.
Streaming vs. non-streaming dispatch and ownership transports/bifrost-http/integrations/router.go	Fallback parsing error handling removes explicit per-branch cancel. Final branching sets streamingOwnsCancel = true before handleStreamingRequest to transfer cancel ownership. Non-streaming requests continue with centralized cancel behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A router once juggled each cancel with care,
Defers scattered everywhere, everywhere!
Now one central defer takes the baton with grace,
While streaming claims ownership of its own space. 🐰✨

✨ 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 05-16-adds_missing_cancel_to_integration_router

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 @coderabbitai help to get the list of available commands and usage tips.}

[akshaydeo]

• adds missing cancel() to integration router #3541 👈 (View in Graphite)
• dev

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

[greptile-apps]

Confidence Score: 4/5

Safe to merge; the consolidation is correct and fixes real resource leaks. The one remaining gap (no cancel on normal stream completion) is a pre-existing design choice and not introduced here.

The centralized defer correctly covers all non-streaming return paths, including the previously unguarded PreCallback error return. The streamingOwnsCancel ownership transfer is clean and matches how handleStreamingRequest already manages cancel() internally. The only open question is whether cancel() should also be called at the end of a fully-successful stream — the current code intentionally omits it, relying on Bifrost's internal cleanup, but the CancelFunc is still technically leaked for the lifetime of the parent context.

transports/bifrost-http/integrations/router.go — specifically the producer goroutine in handleStreaming which never calls cancel() on the happy path.

Important Files Changed

Filename	Overview
transports/bifrost-http/integrations/router.go	Centralizes context cancel management in createHandler; fixes missing cancel() in PreCallback and other early-return paths; uses streamingOwnsCancel flag for clean ownership hand-off to the streaming producer goroutine.

Comments Outside Diff (1)

1. transports/bifrost-http/integrations/router.go, line 2807-2813 (link)

   cancel() never called on successful stream completion

   When a stream completes normally (channel closes without a write error), the producer goroutine exits the for range streamChan loop, optionally sends [DONE], and returns — but never calls cancel(). Because streamingOwnsCancel = true suppresses the outer defer, the CancelFunc returned by ConvertToBifrostContext is permanently leaked for every successful streaming response. The existing comment ("Bifrost handles cleanup internally for normal completion") explains the design intent, but in practice the context derived from ConvertToBifrostContext is never cancelled and the associated resources are not released until the parent context closes (server shutdown). Adding defer cancel() at the top of the producer goroutine would close this gap.

_{Reviews (1): Last reviewed commit: "adds missing cancel() to integration rou..." | Re-trigger Greptile}

[akshaydeo]

Merge activity

• May 16, 1:13 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 16, 1:13 PM UTC: @akshaydeo merged this pull request with Graphite.