openapi-ts icon indicating copy to clipboard operation
openapi-ts copied to clipboard

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

Open Theo-Steiner opened this issue 1 month ago • 7 comments

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?

Theo-Steiner avatar Nov 20 '25 01:11 Theo-Steiner

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

🦋 Changeset detected

Latest commit: c3aba806467ea77551a6c97727c598d330707185

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

changeset-bot[bot] avatar Nov 20 '25 01:11 changeset-bot[bot]

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

vercel[bot] avatar Nov 20 '25 01:11 vercel[bot]

@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

mrlubos avatar Nov 20 '25 02:11 mrlubos

@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 avatar Nov 21 '25 00:11 Theo-Steiner

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

Theo-Steiner avatar Nov 21 '25 01:11 Theo-Steiner

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

jscarle avatar Nov 22 '25 19:11 jscarle

@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 avatar Dec 12 '25 12:12 jscarle