Skip to content

fix: return complete sse event in async iterator (#2641)#3018

Open
Theo-Steiner wants to merge 6385 commits into
hey-api:mainfrom
Theo-Steiner:fix/#2641-return-complete-sse-event-from-generator
Open

fix: return complete sse event in async iterator (#2641)#3018
Theo-Steiner wants to merge 6385 commits into
hey-api:mainfrom
Theo-Steiner:fix/#2641-return-complete-sse-event-from-generator

Conversation

@Theo-Steiner

@Theo-Steiner Theo-Steiner commented Nov 20, 2025

Copy link
Copy Markdown

Closes #2641

Background

The individual results yielded by the async iterator for the SSE client are just the data part of the server sent events.
The type however, suggests that it should be the full event.

Description

This PR changes the returned value to be the full event, including id, event, data & retry.
Since retry is not part of the actual event, it is not included.

Notes

The SDK snapshot tests started failing because they expected the previous function implementation.
Should I update them manually to reflect the new function body or is there a way to regenerate them automatically?

mrlubos and others added 30 commits October 27, 2025 03:34
fix(cli): move cli script to typescript
…hemas

Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
…rrors-method-name-builder

Export Operation type to fix methodNameBuilder TypeScript errors
…h types

Fixed the Valibot plugin to correctly handle additionalProperties with type definitions. Now generates v.record() instead of v.object({}) to prevent silent data loss.

- Modified object.ts to check for any additionalProperties.type (not just 'object')
- Objects with only additionalProperties: generate v.record(v.string(), <valueSchema>)
- Objects with properties AND additionalProperties: generate v.objectWithRest({properties}, <valueSchema>)

Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Updated test snapshots to reflect the corrected behavior for additionalProperties. The fix now properly generates:
- v.record() for objects with only additionalProperties
- v.objectWithRest() for objects with both properties and additionalProperties

This correctly handles free-form objects per OpenAPI spec defaults.

Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@changeset-bot

changeset-bot Bot commented Nov 20, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c3aba80

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Nov 20, 2025

Copy link
Copy Markdown

@Theo-Steiner is attempting to deploy a commit to the Hey API Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug 🔥 Broken or incorrect behavior. client labels Nov 20, 2025
@mrlubos

mrlubos commented Nov 20, 2025

Copy link
Copy Markdown
Member

@Theo-Steiner run pnpm test:update to update snapshots. However, I'm confused by this issue because server-sent events are working in the Opencode example https://github.com/hey-api/openapi-ts/blob/main/packages/openapi-ts-tests/sdks/__snapshots__/opencode/flat/sdk.gen.ts#L24. I worry making this change would break that functionality. To me it sounds like there's something else going on

@Theo-Steiner

Theo-Steiner commented Nov 21, 2025

Copy link
Copy Markdown
Author

@mrlubos thank you for your quick reply!
The opencode example you linked seems to only check whether you can connect to a server sent event source. This works without problems already.
The issue is that the stream property returned by sse.get() contains only the data, whereas the types say it would contain the entire event.

// ASIS
export const { stream } = getSse();

for await (const result of stream) {
  // `result` is typed as `GetSseResponses`, but the actual result is `GetSseResponses['data']`
  console.log(result.event) // event will always be undefined, since we are passed only the data
}

// TOBE
for await (const result of stream) {
  // `result` is still typed as `GetSseResponses`, however the actual result is now the complete event (not just it's data property)
  console.log(result.event) // logs the actual event
}

As you can see, there is currently a bug in the SSE implementation, where the result.data property is returned when the types promise the entire result object should be returned instead.
As far as I can see it, there's two ways to fix it:

  1. adjust the yielded value to match the types (this PR)
  2. change the types to match the yielded value (this would make the stream property of the sse results basically useless, since the data is not really useful without the event type)

@Theo-Steiner

Theo-Steiner commented Nov 21, 2025

Copy link
Copy Markdown
Author

I've updated the tests, which sadly bloats the diff of this PR by quite a bit 😅
also looks like pnpm examples:generate doesn't update the examples anymore..?

@jscarle

jscarle commented Nov 22, 2025

Copy link
Copy Markdown

❤️ I'm so looking forward to this. Thanks!

@mrlubos mrlubos removed the client label Dec 11, 2025
@jscarle

jscarle commented Dec 12, 2025

Copy link
Copy Markdown

@mrlubos Any chance we can look at this as it seriously limits the use of SSE without having access to event name and ID...

@jscarle

jscarle commented Jan 10, 2026

Copy link
Copy Markdown

@mrlubos Can we take a look at this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🔥 Broken or incorrect behavior. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AsyncGenerator of a text/event-stream returns the data only

7 participants