Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 29 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Coverage Report
File CoverageNo changed files found. |
TSavo
force-pushed
the
fix/sse-streaming-debit
branch
from
a1a45c7 to
8518805
Compare
Greptile SummaryThis PR fixes a billing gap where SSE streaming responses were metering usage (emitting meter events) but never actually debiting the tenant's credit balance. The fix adds a The change is correct in intent and the arguments ( Key concern:
Confidence Score: 3/5Not safe to merge as-is — the unawaited debit in The fix addresses the right problem (missing debit on SSE path) but introduces a reliability risk: src/gateway/streaming.ts — the
|
| Filename | Overview |
|---|---|
| src/gateway/streaming.ts | Adds the missing debitCredits call in flush() to fix SSE billing, but the call is fire-and-forget (unawaited) in an async context — the stream closes before the ledger write completes, which can silently drop debits in edge/serverless runtimes. |
Sequence Diagram
sequenceDiagram
participant Client
participant TransformStream
participant flush as flush()
participant debitCredits
participant Ledger
Client->>TransformStream: SSE chunks (transform x N)
TransformStream->>TransformStream: accumulate cost
TransformStream->>flush: [DONE] received → flush()
flush->>flush: meter.emit(cost, charge)
flush->>debitCredits: debitCredits(...) [NOT awaited ⚠️]
flush-->>TransformStream: flush() resolves (stream closes)
Note over Client,TransformStream: Response already committed
debitCredits->>Ledger: creditLedger.debit() [may be dropped!]
Ledger-->>debitCredits: ok
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/gateway/streaming.ts
Line: 124-125
Comment:
**Unawaited debit in async `flush()` — credits can be silently dropped**
`debitCredits` returns a `Promise<void>` that is not awaited. Because `flush()` is already `async`, the TransformStream resolves its flush contract (closing the stream) before the debit finishes executing. In serverless or edge runtimes (Cloudflare Workers, Lambda), the isolate may be terminated as soon as the HTTP response is committed, silently dropping the pending debit — meaning the tenant gets the tokens for free.
The non-streaming path shares this pattern, but the risk is higher here: the streaming response _closes_ at the moment `flush()` settles, which is before `debitCredits` awaits the ledger. Since `flush()` is async and the debit already handles its own errors internally, awaiting it adds zero performance cost and guarantees billing integrity before the stream terminates.
```suggestion
// Debit credits — await to guarantee the ledger write completes before the stream closes
await debitCredits(deps, tenant.id, accumulatedCost, deps.defaultMargin, capability, provider);
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: debit credits in SSE streaming path..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Debit credits (fire-and-forget, same as non-streaming path) | ||
| debitCredits(deps, tenant.id, accumulatedCost, deps.defaultMargin, capability, provider); |
There was a problem hiding this comment.
flush() — credits can be silently dropped
debitCredits returns a Promise<void> that is not awaited. Because flush() is already async, the TransformStream resolves its flush contract (closing the stream) before the debit finishes executing. In serverless or edge runtimes (Cloudflare Workers, Lambda), the isolate may be terminated as soon as the HTTP response is committed, silently dropping the pending debit — meaning the tenant gets the tokens for free.
The non-streaming path shares this pattern, but the risk is higher here: the streaming response closes at the moment flush() settles, which is before debitCredits awaits the ledger. Since flush() is async and the debit already handles its own errors internally, awaiting it adds zero performance cost and guarantees billing integrity before the stream terminates.
| // Debit credits (fire-and-forget, same as non-streaming path) | |
| debitCredits(deps, tenant.id, accumulatedCost, deps.defaultMargin, capability, provider); | |
| // Debit credits — await to guarantee the ledger write completes before the stream closes | |
| await debitCredits(deps, tenant.id, accumulatedCost, deps.defaultMargin, capability, provider); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/streaming.ts
Line: 124-125
Comment:
**Unawaited debit in async `flush()` — credits can be silently dropped**
`debitCredits` returns a `Promise<void>` that is not awaited. Because `flush()` is already `async`, the TransformStream resolves its flush contract (closing the stream) before the debit finishes executing. In serverless or edge runtimes (Cloudflare Workers, Lambda), the isolate may be terminated as soon as the HTTP response is committed, silently dropping the pending debit — meaning the tenant gets the tokens for free.
The non-streaming path shares this pattern, but the risk is higher here: the streaming response _closes_ at the moment `flush()` settles, which is before `debitCredits` awaits the ledger. Since `flush()` is async and the debit already handles its own errors internally, awaiting it adds zero performance cost and guarantees billing integrity before the stream terminates.
```suggestion
// Debit credits — await to guarantee the ledger write completes before the stream closes
await debitCredits(deps, tenant.id, accumulatedCost, deps.defaultMargin, capability, provider);
```
How can I resolve this? If you propose a fix, please make it concise.
|
🎉 This PR is included in version 1.75.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
SSE streams were metering but never debiting credits. Added debitCredits call in streaming.ts flush().
Note
Fix credit debit in SSE streaming path on stream completion
The SSE streaming path was not debiting tenant credits after stream completion. Adds a fire-and-forget call to
debitCreditsinproxySSEStreamusing the accumulated stream cost and default margin after metering data is emitted.Risk: Credit deduction now occurs on every SSE stream completion where it was previously skipped entirely.
Macroscope summarized 8518805.
Summary by CodeRabbit