Repository navigation
feat(typespec-ts): streaming - #5196
Libba Lawrence (l0lawrence) wants to merge 36 commits into
Conversation
…ault JSONL/SSE stream responses now return Promise<AsyncIterable<T>> of deserialized items/events instead of the raw binary Uint8Array body. JsonlStream<T> lazily decodes JSON Lines; SSEStream<T> dispatches each Server-Sent Event by its event: name (via @azure/core-sse), deserializes the payload, and stops at the terminal event. Heterogeneous multi-event dispatch and terminal-event handling are supported. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Adds the workspace link entries for @typespec/streams, @typespec/sse and @typespec/events that were added to packages/typespec-ts/package.json for structured JSONL/SSE streaming. Required so CI's frozen-lockfile install matches package.json. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
|
All changed packages have been documented.
Show changes
|
Brings the branch up to date with upstream main so CI can build the PR merge commit. Resolves the conflict in pnpm-lock.yaml (took upstream and re-added the @typespec/streams, @typespec/sse and @typespec/events workspace links) and adapts the structured-streaming deserialize helper to the refactored getExceptionThrowStatement signature (now [string[], ServiceOperation]). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
📦 Package size report6 packages changed size, -836 B (-0.0%) packed overall.
8 package(s) with no notable change
Packed = gzipped |
commit: |
|
You can try these changes here
|
Three fixes from a design review of the default JSONL/SSE structured streaming feature: 1. Split the SSE decoder into a separate sseStreamingHelpers.ts so JSONL-only packages no longer import (or need) @azure/core-sse. streamingHelpers.ts now contains only JSONL + shared helpers with no core-sse dependency. SseStreamingHelpers loads only when the package has an SSE operation, matching the @azure/core-sse dependency gate. 2. Only treat a constant SSE event as a terminal sentinel when isTerminalEvent is true. A non-terminal constant is a real payload variant and is now deserialized/yielded and included in the item type instead of silently terminating the stream. 3. Thread the event payloadContentType into each SSE descriptor and only JSON.parse JSON payloads; non-JSON (e.g. text/plain) payloads are passed through as the raw data string. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Opt the http-specs streaming/sse scenario (unnamed, named+terminal, retrieve) into the typespec-ts integration suite so the default structured-streaming codegen is exercised end-to-end against the real spector mock. The generated public surface confirms the operations return Promise<AsyncIterable<T>> with heterogeneous per-event dispatch and terminal-event handling. JSONL is deferred from the integration baseline: its send() op exercises request-body (upload) stream typing, a pre-existing emitter gap unrelated to this response-streaming feature. JSONL response streaming remains covered by the unit-modular snapshot. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Remove unrelated @typespec/http-client-python 0.35.1 churn from the lock and keep only the @typespec/events, @typespec/sse, and @typespec/streams workspace links required by typespec-ts's new streaming dependencies. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Add @azure/core-sse devDependency (with clean lockfile entries) and a runtime integration test that exercises the generated SSE clients against the http-specs mock server: unnamed message events, heterogeneous named events with terminal [DONE] handling, and event-name-dispatched retrieve streaming. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Move the @azure/core-sse version into the pnpm-workspace.yaml catalog and reference it via catalog: in package.json, matching the repo policy that all external dependencies use the catalog protocol. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Merge jsonlStreaming.md, sseUnnamedStreaming.md, and sseNamedStreaming.md into a single streaming.md with three scenarios (the scenario runner splits each file on H1, so one file holds multiple scenarios). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
…turedStreamInfo Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
…ating Two structured-streaming correctness fixes: - Non-terminal SSE events whose payload needs no model deserializer (primitive/scalar/enum) were silently dropped because no deserialize callback was emitted. Emit an identity deserializer so the raw payload (JSON-parsed value, or raw string for non-JSON content) is yielded. Add a regression scenario mixing a model and a text/plain scalar event. - packageHasSseStreaming now mirrors getStructuredStreamInfo's gating (paging/LRO exclusions + streamMetadata & sseMetadata requirement) so @azure/core-sse is only injected when SSE streaming is actually generated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Maor Leger (maorleger)
left a comment
There was a problem hiding this comment.
This is coming along quite nicely!! Thanks so much for all the effort in getting this together.
I still have lots to review, but the overall direction seems ok. I noted a few findings that we should fix though
- Add streamingHelpers-browser.mts: uses asBrowserStream() and converts ReadableStream to AsyncIterable - Add streamingHelpers-react-native.mts: re-exports browser variant - Add sseStreamingHelpers-browser.mts: re-exports base (body already AsyncIterable) - Add sseStreamingHelpers-react-native.mts: re-exports browser variant - Fix terminal event name-aware matching in sseStreamingHelpers.ts (check descriptor by name first) - Add content-type validation for JSONL vs SSE stream detection - Add named event type discrimination for SSE return types (discriminated union) - Update streaming test baselines for named event discriminated unions - Clarify in SSE integration test that discriminated union enables type-safe narrowing Follows the same pattern as get-binary-response polyfills. Enables: - Node.js: StreamableMethod.asNodeStream() → AsyncIterable (default) - Browser: StreamableMethod.asBrowserStream() → ReadableStream → AsyncIterable - React-Native: Same as browser (browser APIs available) Addresses PR review comments: - r3815393091: Environment polyfills - r3815434566: Content-type validation - r3815531940: Terminal event name-aware matching - r3815653900: Named event type discrimination with discriminated union Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5de9bff8-f217-4c66-9e30-bdc80256fed8
c503c07 to
1a2adc2
Compare
Include the SSE event name when JSON payload parsing fails and preserve the original error as the cause. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5de9bff8-f217-4c66-9e30-bdc80256fed8
Generate discriminated union return types for heterogeneous named SSE streams so callers can narrow payloads by event name. Update streaming operation baselines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5de9bff8-f217-4c66-9e30-bdc80256fed8
…ed events in an envelope
Terminal events are now always consumed internally and never yielded, so the
declared item union is sound by construction. The terminal check runs after
descriptor resolution by event name (an unrelated event carrying the sentinel
data can no longer end the stream) and before deserialization (terminal
payloads are never yielded).
Named SSE events are wrapped in a { event, data } envelope so the generated
return type matches what the runtime helper yields. The envelope is applied
all-or-nothing via everyPayloadEventIsNamed to avoid a union mixing envelope
and bare shapes.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6a6ea9-d1f8-4a50-831f-d64d9f7cfa0d
Use ordinary JSON.stringify quoting instead of custom script-embedding escapes. Remove the dedicated utility and exotic-metadata tests while preserving SSE Spector coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bbec15a-04ed-41e4-9fae-e27932b80317
Include terminal constants in generated return types and yield them before stopping. Preserve named event envelopes with the default message name for unnamed terminals, keep unnamed streams payload-only, and update runtime, snapshots and Spector coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bbec15a-04ed-41e4-9fae-e27932b80317
| const yielded = useEventEnvelope | ||
| ? `({ event: ${JSON.stringify(event.eventName ?? "message")}, data: ${event.deserializeExpression} })` | ||
| : event.deserializeExpression; | ||
| parts.push(`deserialize: (data) => ${yielded}`); |
There was a problem hiding this comment.
I don't think this will be an issue?
There was a problem hiding this comment.
🟡 Changes recommended
Critical browser SSE cleanup and scalar deserialization issues remain, along with JSONL gating and breaking-release metadata corrections.
3 open findings
13 resolved since last review
Deserialize scalar SSE payloads instead of returning raw strings Regenerate lockfile to satisfy the @azure/core-sse version range Handle unnamed SSE frames with the fallback descriptor Fix browser helper import to resolve the emitted JavaScript module Replace nonexistent SseStream type in generated SSE helper Replace nonexistent SseStream type in browser helper Use an available SSE stream API or implement reconnecting wrappergetStreamResponseleavesresult.bodyas anAsyncIterable, butgetExceptionThrowStatement… This path ignoressseEvent.isEventEnvelopeand deserializespayloadTypedirectly. For an event… The PR adds substantial JSONL decoding logic (line splitting, buffering across chunks, JSON.parse… On early termination (for example, after an SSE terminal event), this generator'sfinallyonly… An omittedpayloadContentTypeis not always JSON: TCGC leaves it undefined for inferred event… The new JSONL decoder has no runtime unit or integration coverage; the added helper tests exercise…
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
🧠 Review effort: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bbec15a-04ed-41e4-9fae-e27932b80317
There was a problem hiding this comment.
🔵 Needs a closer look
Browser SSE terminal cleanup fails with the pinned dependency, and structured-stream response aliases are misleading.
0 open findings
3 resolved since last review
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
🧠 Review effort: Lite
| event: EventMessage, | ||
| descriptors: SseEventDescriptor<T>[], | ||
| ): SseEventDescriptor<T> | undefined { | ||
| if (event.event) { |
There was a problem hiding this comment.
can event.event be empty string? Other falsy values would also reach the else block.
There was a problem hiding this comment.
we initialize it to be "" so yes it can be that gets treated as eventType "message"
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bbec15a-04ed-41e4-9fae-e27932b80317
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bbec15a-04ed-41e4-9fae-e27932b80317
…tream on terminal event Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bbec15a-04ed-41e4-9fae-e27932b80317
| - "@azure-tools/typespec-ts" | ||
| --- | ||
|
|
||
| Generate structured JSONL and SSE streaming operations as `Promise<AsyncIterable<T>>` of deserialized items/events instead of raw binary `Uint8Array` bodies. Connection and modeled HTTP error validation are eager; payload deserialization is lazy. Generate streaming helpers only for supported MIME types and non-paging, non-LRO operations. Deserialize scalar SSE payloads into their declared client types. Dispatch unnamed discriminated SSE model variants by their payload discriminator and yield terminal payloads before stopping. |
There was a problem hiding this comment.
The content here will be shown in the changelog once a new version is released. I suggest that we make it shorter.
| ? event.deserializeExpression | ||
| : String(sseEvent.payloadType.value); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Is it possible that an event be nullable (sseEvent.payloadType.kind === "nullable")? For example:
model Payload {
@encodedName("application/json", "wire_value")
value: string;
timestamp: utcDateTime;
}
@events
union NullableEvents {
@Events.contentType("application/json")
update: Payload | null,
}
@route("/receive")
op receive(): SSEStream<NullableEvents>;
The current deserializing behavior is
deserialize: (data) => ({
event: "update",
data: data === null ? null : data, // <- should be payloadDeserializer(data)
}),
I think we should still use the deserializer if data is not null.
There was a problem hiding this comment.
Let me verify but I don't think there is a linter preventing it and if the wire data could be data: null
| isAsync: true, | ||
| isExported: true, | ||
| name, | ||
| propertyName: normalizeName(operation.name, NameType.Property), |
There was a problem hiding this comment.
propertyName names the public method on the client or operation group, and it should come from getOperationName as well, like name. Otherwise, a method annotated with @clientName(exact("my_operation")) will become myOperation at the client level.
| throw createRestError(result); | ||
| } | ||
|
|
||
| const contentType_1 = Object.entries(response.headers) |
There was a problem hiding this comment.
Just curious, why is it contentType_1, not contentType?
There was a problem hiding this comment.
there was a contentType in here already so defautled to _1 - let me see about rename
| } else if (descriptor.discriminator) { | ||
| discriminated.push(descriptor); | ||
| } else { | ||
| fallback ??= descriptor; |
There was a problem hiding this comment.
If variants are unnamed, nonterminal constants, like below:
@events
union ConstantEvents {
@Events.contentType("text/plain")
"cat",
@Events.contentType("text/plain")
"dog",
}
@route("/receive")
op receive(): SSEStream<ConstantEvents>;
The generated descriptors would be:
[
{ isTerminal: false, deserialize: () => "cat", contentType: "text/plain" },
{ isTerminal: false, deserialize: () => "dog", contentType: "text/plain" },
]
Will runtime fallback to the first descriptor no matter the response is "cat" or "dog"?
There was a problem hiding this comment.
it would, can add something to check for value to avoid that? right now I dont believe search uses unnamed events at all so we shouldn't run into this scenario yet at least
| ?.split(";", 1)[0] | ||
| .trim() | ||
| .toLowerCase(); | ||
| if (contentType !== "text/event-stream" || !response.body) { |
There was a problem hiding this comment.
If the operation is declared like
@route("/receive")
op receive(): SSEStream<MessageEvents> | NoContentResponse;
which has a streaming response and a 204 response, will an error be thrown if response.body is empty which is not expected?
There was a problem hiding this comment.
ohh I think it would, let me fix that



Adds structured JSONL + SSE streaming code generation to the
@azure-tools/typespec-tsmodular emitter. A response whose TCGC metadata indicates a structured stream now generates aPromise<AsyncIterable<T>>of deserialized items/events instead of returning the raw binaryUint8Arraybody. Does not contain reconnection support - depends on createStreamSSE(). Azure Search, the library requesting this feature, does not support reconnection at this point in time.The sse spec this is based on: https://html.spec.whatwg.org/multipage/server-sent-events.html
What is generated
JsonlStream<T>) ->Promise<AsyncIterable<T>>that lazily decodes JSON Lines.SSEStream<T>) ->Promise<AsyncIterable<...>>that dispatches each Server-Sent Event by itsevent:name (via@azure/core-sse), deserializes the payload, and stops at the terminal event. Full heterogeneous multi-event dispatch and@terminalEventhandling are supported (named terminals keyed on the terminal flag; unnameddata: [DONE]sentinel terminals). Generation is driven by TCGCstreamMetadata/sseMetadata.Issues for the future:
#5660 #5659
Related pr to core for abort signal/canceling stream: Azure/azure-sdk-for-js#40238