fix: handle errors which occur prior to stream initiation #57
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Danny Kopping <[email protected]>
…thropic only) Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
dannykopping
commented
Nov 13, 2025
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
dannykopping
force-pushed
the
non-stream-err-handling
branch
from
c57a572 to
d94d846
Compare
dannykopping
commented
Nov 14, 2025
| return | ||
| } | ||
|
|
||
| // Initiate the stream once the first event is received. |
Collaborator
Author
There was a problem hiding this comment.
This is the main fix, besides the provider-specific implementations.
We can't send the SSE headers until we know whether the stream has started. If the upstream request fails before streaming starts, we have to send back a non-200 response. Once SSE headers are sent, we can't do that.
pawbana
requested changes
Nov 17, 2025
Signed-off-by: Danny Kopping <[email protected]>
pawbana
approved these changes
Nov 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #56
Also introduces a new test struct named
mockHTTPReflectorwhich reflects an entire HTTP response fixture. This should be the approach going forward, instead of the somewhat jankymockServer. You'll notice in the modified fixtures that they contain a full HTTP payload instead of just the response body. I think this makes it a lot cleaner and more idiomatic.