[core-client-rest] Implement environment checks for streaming methods #33138
Conversation
|
API change check API changes are not detected in this pull request. |
| { ...options, allowInsecureConnection, responseAsStream: true }, | ||
| httpClient, | ||
| ) as Promise<HttpBrowserStreamResponse>; | ||
| if (isNodeLike) { |
There was a problem hiding this comment.
probably for future consideration: it would be nice if we could do feature detection of whether web stream API is available, if the support is added later
There was a problem hiding this comment.
Yeah, that would be nice. That may be slightly more involved since the stream comes directly from the HTTP client. Maybe we can eventually use Readable.toWeb, but last time I played around with that API I found it to be a bit unstable. That was over a year ago though so maybe the situation has improved a little.
There was a problem hiding this comment.
In the error message could mention toWeb as something they can try? Looking at the Node docs it doesn't look like we'll be able to depend on webstreams until 22 becomes out minimum (ETA summer 2026 if I'm reading this right)
| asNodeStream: () => Promise<HttpNodeStreamResponse>; | ||
| /** | ||
| * Returns the response body as a browser stream. Only available in the browser. | ||
| */ | ||
| asBrowserStream: () => Promise<HttpBrowserStreamResponse>; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Sure, we can certainly consider this as a feature in future, and I would support doing that. But in the meantime this fix resolves the immediate issue: the type claims that asBrowserStream returns a Web stream in Node-like environments where it actually returns a Node stream.
Packages impacted by this PR
@azure-rest/core-client@typespec/ts-http-runtimeIssues associated with this PR
Describe the problem that is addressed by this PR
The result of calling
asNodeStreamin the browser and of callingasBrowserStreamin Node is has the incorrect type. Since these methods were only ever intended to be called in their corresponding environments, this PR updates them to throw an error if they are called in the wrong environment. Also updated docs to clarify.