fix(pearl): correct SSE error event format and fix infinite loading o…#357
fix(pearl): correct SSE error event format and fix infinite loading o…#357HARSH048 wants to merge 1 commit intobubblelabai:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdjusted streaming SSE behavior and payload shape: removed an ignored-event comment, added a frontend safety cleanup to cancel unfinished generation, and unified backend SSE Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/bubblelab-api/src/routes/bubble-flows.ts (1)
1421-1446:⚠️ Potential issue | 🟡 MinorAvoid emitting two error signals for the same failure.
When
coffeeResult.success === false, the backend now sends anerrorSSE event and then astream_completeevent that still carries the failedcoffeeResult. On the frontend, bothcase 'error'(lines 410-422 inusePearlChatStore.ts) andcase 'stream_complete'(lines 429-445, newly added) independently callstate.addEvent({ type: 'generation_error', ... })andstate.setGenerationCompleted(true). This will duplicate the error in the timeline UI.Pick one source of truth — either stop including the failing
coffeeResultinstream_completeafter emitting theerrorevent, or drop thecoffeeResultbranch from the frontendstream_completehandler now thaterroris properly emitted.🔧 Proposed server-side fix
if (!coffeeResult.success) { await stream.writeSSE({ data: JSON.stringify({ type: 'error', data: { error: coffeeResult.error || 'Flow generation failed. Please check your API key configuration.', recoverable: false, }, }), event: 'error', }); } await stream.writeSSE({ data: JSON.stringify({ type: 'stream_complete', timestamp: new Date().toISOString(), - coffeeResult, + ...(coffeeResult.success ? { coffeeResult } : {}), }), event: 'stream_complete', });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/bubblelab-api/src/routes/bubble-flows.ts` around lines 1421 - 1446, The backend is emitting both an 'error' SSE and then a 'stream_complete' SSE that still includes the failing coffeeResult, causing duplicate error handling client-side; update the code around the coffeeResult handling in the bubble-flows route so that when coffeeResult.success === false you still emit the 'error' SSE via stream.writeSSE(...) but send the subsequent 'stream_complete' SSE without the failing coffeeResult (e.g., omit the coffeeResult field or set it null/undefined) so the frontend's stream_complete handler no longer treats it as an additional error; ensure the 'error' branch uses the existing event payload and only the successful path includes coffeeResult in the stream_complete payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/bubble-studio/src/hooks/usePearlChatStore.ts`:
- Around line 429-445: The 'stream_complete' branch currently duplicates
handling already done in the 'error' SSE handler and blindly casts event to read
coffeeResult; remove the coffeeResult failure branch inside the 'case
"stream_complete"' or guard it so it only runs when no prior 'error' event was
received (e.g., track a local flag like errorEventReceived set by the existing
'case "error"' handler before calling
state.addEvent/state.setGenerationCompleted). Also avoid the blind cast by using
the shared StreamingEvent type (or extend that schema to include coffeeResult)
when reading coffeeResult to keep typings correct.
---
Outside diff comments:
In `@apps/bubblelab-api/src/routes/bubble-flows.ts`:
- Around line 1421-1446: The backend is emitting both an 'error' SSE and then a
'stream_complete' SSE that still includes the failing coffeeResult, causing
duplicate error handling client-side; update the code around the coffeeResult
handling in the bubble-flows route so that when coffeeResult.success === false
you still emit the 'error' SSE via stream.writeSSE(...) but send the subsequent
'stream_complete' SSE without the failing coffeeResult (e.g., omit the
coffeeResult field or set it null/undefined) so the frontend's stream_complete
handler no longer treats it as an additional error; ensure the 'error' branch
uses the existing event payload and only the successful path includes
coffeeResult in the stream_complete payload.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ecf1312-e7bb-49b3-be69-e87e27f092d1
📒 Files selected for processing (4)
apps/bubble-studio/src/hooks/usePearlChatStore.tsapps/bubble-studio/src/hooks/usePearlStream.tsapps/bubblelab-api/src/routes/ai.tsapps/bubblelab-api/src/routes/bubble-flows.ts
| case 'stream_complete': { | ||
| // Check if the backend returned an error result (e.g. missing API key) | ||
| const streamData = event as unknown as { | ||
| coffeeResult?: { type?: string; error?: string; success?: boolean }; | ||
| }; | ||
| if ( | ||
| streamData.coffeeResult && | ||
| streamData.coffeeResult.success === false | ||
| ) { | ||
| const errorMsg = | ||
| streamData.coffeeResult.error || | ||
| 'Generation failed. Please check your API key configuration.'; | ||
| state.addEvent({ type: 'generation_error', message: errorMsg }); | ||
| state.setGenerationCompleted(true); // also clears abort controller + isGenerating | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
Redundant with the new error event — risks double-logging the failure.
The backend now emits a proper error SSE event before stream_complete when coffeeResult.success === false (see bubble-flows.ts lines 1423-1436). The existing case 'error' handler (lines 410-422) already calls state.addEvent({ type: 'generation_error', ... }) and state.setGenerationCompleted(true), so this block will add a second generation_error timeline event and re-trigger setGenerationCompleted for the same failure.
Recommended: remove this stream_complete error branch (the error event is now the single source of truth), or guard it so it only fires when no prior error event was received in this run.
Additionally, the coffeeResult field is read from the root of the event via event as unknown as { coffeeResult?: ... }, which bypasses StreamingEvent typing. If you keep any logic here, consider extending the shared schema so this isn't a blind cast.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/bubble-studio/src/hooks/usePearlChatStore.ts` around lines 429 - 445,
The 'stream_complete' branch currently duplicates handling already done in the
'error' SSE handler and blindly casts event to read coffeeResult; remove the
coffeeResult failure branch inside the 'case "stream_complete"' or guard it so
it only runs when no prior 'error' event was received (e.g., track a local flag
like errorEventReceived set by the existing 'case "error"' handler before
calling state.addEvent/state.setGenerationCompleted). Also avoid the blind cast
by using the shared StreamingEvent type (or extend that schema to include
coffeeResult) when reading coffeeResult to keep typings correct.
…n missing API key
Three related bugs fixed:
1. Wrong SSE error event shape in backend routes
- bubble-flows.ts and ai.ts were sending { type: 'error', error: '...' }
- StreamingEvent schema requires { type: 'error', data: { error, recoverable } }
- Frontend handler reads event.data.error, so errors silently fell back to
'An error occurred' or were swallowed entirely
2. Missing error event on coffee agent failure (planning phase)
- When GOOGLE_API_KEY is absent, coffee.ts returns early with success:false
- The route only sent stream_complete with the error buried in coffeeResult
- No proper error SSE event was emitted, so the frontend never received it
3. UI stuck in infinite loading state when API key is missing
- generationAbortController was never cleared after the stream ended
- isGenerating stayed true forever since setGenerationCompleted was never called
- Added safety net in startGenerationStream to always clear isGenerating after
the stream loop exits without a generation_complete event
- error event is now the single source of truth; stream_complete is a no-op
HARSH048
force-pushed
the
fix/sse-error-event-format-and-infinite-loading
branch
from
ae5de08 to
2f9a0e9
Compare
1 participant
…n missing API key
Three related bugs fixed:
Wrong SSE error event shape in backend routes
Missing error event on coffee agent failure (planning phase)
UI stuck in infinite loading state when API key is missing
Summary
Related Issues
Type of Change
Checklist
pnpm checkand all tests passScreenshots (Required)
For New Bubble Integrations
.integration.flow.ts) covers all operationsAdditional Context
Summary by CodeRabbit