From a8327cb974bed333f501addd2690cc5677899471 Mon Sep 17 00:00:00 2001 From: Jose Manuel Heredia Hidalgo Date: Tue, 2 Nov 2021 10:42:24 -0700 Subject: [PATCH 1/4] Add responseAsStream to PipelineRequest --- .../review/core-rest-pipeline.api.md | 2 ++ sdk/core/core-rest-pipeline/src/interfaces.ts | 5 +++++ sdk/core/core-rest-pipeline/src/nodeHttpClient.ts | 2 +- .../core-rest-pipeline/src/pipelineRequest.ts | 8 ++++++++ sdk/core/core-rest-pipeline/src/xhrHttpClient.ts | 5 +++-- .../test/browser/xhrHttpClient.spec.ts | 15 +++++++++++++++ .../test/node/nodeHttpClient.spec.ts | 15 +++++++++++++++ 7 files changed, 49 insertions(+), 3 deletions(-) diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index 5a98fd4448ab..00a523abc1ab 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -199,6 +199,7 @@ export interface PipelineRequest { onUploadProgress?: (progress: TransferProgressEvent) => void; proxySettings?: ProxySettings; requestId: string; + responseAsStream?: boolean; streamResponseStatusCodes?: Set; timeout: number; tracingOptions?: OperationTracingOptions; @@ -219,6 +220,7 @@ export interface PipelineRequestOptions { onUploadProgress?: (progress: TransferProgressEvent) => void; proxySettings?: ProxySettings; requestId?: string; + responseAsStream?: boolean; streamResponseStatusCodes?: Set; timeout?: number; tracingOptions?: OperationTracingOptions; diff --git a/sdk/core/core-rest-pipeline/src/interfaces.ts b/sdk/core/core-rest-pipeline/src/interfaces.ts index 93e8a469a5df..ed2acdb13158 100644 --- a/sdk/core/core-rest-pipeline/src/interfaces.ts +++ b/sdk/core/core-rest-pipeline/src/interfaces.ts @@ -141,6 +141,11 @@ export interface PipelineRequest { */ streamResponseStatusCodes?: Set; + /** + * If set, body will be returned as a raw stream. + */ + responseAsStream?: boolean; + /** * Proxy configuration. */ diff --git a/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts b/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts index 60f22ffc3252..64eef6eaa9de 100644 --- a/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts +++ b/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts @@ -151,7 +151,7 @@ class NodeHttpClient implements HttpClient { responseStream = downloadReportStream; } - if (request.streamResponseStatusCodes?.has(response.status)) { + if (request.responseAsStream || request.streamResponseStatusCodes?.has(response.status)) { response.readableStreamBody = responseStream; } else { response.bodyAsText = await streamToText(responseStream); diff --git a/sdk/core/core-rest-pipeline/src/pipelineRequest.ts b/sdk/core/core-rest-pipeline/src/pipelineRequest.ts index e3364325c404..c249454f2177 100644 --- a/sdk/core/core-rest-pipeline/src/pipelineRequest.ts +++ b/sdk/core/core-rest-pipeline/src/pipelineRequest.ts @@ -68,6 +68,11 @@ export interface PipelineRequestOptions { */ streamResponseStatusCodes?: Set; + /** + * If set, body will be returned as a raw stream. + */ + responseAsStream?: boolean; + /** * Proxy configuration. */ @@ -109,6 +114,8 @@ class PipelineRequestImpl implements PipelineRequest { public body?: RequestBodyType; public formData?: FormDataMap; public streamResponseStatusCodes?: Set; + public responseAsStream?: boolean; + public proxySettings?: ProxySettings; public disableKeepAlive: boolean; public abortSignal?: AbortSignalLike; @@ -128,6 +135,7 @@ class PipelineRequestImpl implements PipelineRequest { this.disableKeepAlive = options.disableKeepAlive ?? false; this.proxySettings = options.proxySettings; this.streamResponseStatusCodes = options.streamResponseStatusCodes; + this.responseAsStream = options.responseAsStream; this.withCredentials = options.withCredentials ?? false; this.abortSignal = options.abortSignal; this.tracingOptions = options.tracingOptions; diff --git a/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts b/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts index 7fdaee72c57e..3ea7be248073 100644 --- a/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts +++ b/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts @@ -67,7 +67,8 @@ class XhrHttpClient implements HttpClient { for (const [name, value] of request.headers) { xhr.setRequestHeader(name, value); } - xhr.responseType = request.streamResponseStatusCodes?.size ? "blob" : "text"; + xhr.responseType = + request.responseAsStream || request.streamResponseStatusCodes?.size ? "blob" : "text"; if (isReadableStream(request.body)) { throw new Error("Node streams are not supported in browser environment."); @@ -105,7 +106,7 @@ function handleBlobResponse( xhr.addEventListener("readystatechange", () => { // Resolve as soon as headers are loaded if (xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED) { - if (request.streamResponseStatusCodes?.has(xhr.status)) { + if (request.responseAsStream || request.streamResponseStatusCodes?.has(xhr.status)) { const blobBody = new Promise((resolve, reject) => { xhr.addEventListener("load", () => { resolve(xhr.response); diff --git a/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts b/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts index d57d8cc8aad7..15706eb0a01f 100644 --- a/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts +++ b/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts @@ -153,6 +153,21 @@ describe("XhrHttpClient", function() { assert.ok(response.blobBody, "Expect streaming body"); }); + it("should stream response body on responseAsStream", async function() { + const client = createDefaultHttpClient(); + const request = createPipelineRequest({ + url: "https://example.com", + responseAsStream: true + }); + const promise = client.sendRequest(request); + assert.equal(requests.length, 1); + requests[0].respond(200, {}, "body"); + const response = await promise; + assert.strictEqual(response.status, 200); + assert.equal(response.bodyAsText, undefined); + assert.ok(response.blobBody, "Expect streaming body"); + }); + it("should not stream response body on non-matching status code", async function() { const client = createDefaultHttpClient(); const request = createPipelineRequest({ diff --git a/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts b/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts index 7fcd4eb343b9..48264da5e236 100644 --- a/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts +++ b/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts @@ -194,6 +194,21 @@ describe("NodeHttpClient", function() { assert.ok(response.readableStreamBody); }); + it("should stream response body on responseAsStream", async function() { + const client = createDefaultHttpClient(); + const clientRequest = createRequest(); + stubbedHttpsRequest.returns(clientRequest); + const request = createPipelineRequest({ + url: "https://example.com", + responseAsStream: true + }); + const promise = client.sendRequest(request); + stubbedHttpsRequest.yield(createResponse(200, "body")); + const response = await promise; + assert.equal(response.bodyAsText, undefined); + assert.ok(response.readableStreamBody); + }); + it("should not stream response body on non-matching status code", async function() { const client = createDefaultHttpClient(); const clientRequest = createRequest(); From 81cd72ecd63537b14a05dc52c2fa513ceb423458 Mon Sep 17 00:00:00 2001 From: Jose Manuel Heredia Hidalgo Date: Tue, 2 Nov 2021 12:39:35 -0700 Subject: [PATCH 2/4] Use Number.POSITIVE_INFINITY to indicate that any status code should get raw response --- .../review/core-rest-pipeline.api.md | 2 -- sdk/core/core-rest-pipeline/src/interfaces.ts | 6 +----- sdk/core/core-rest-pipeline/src/nodeHttpClient.ts | 6 +++++- sdk/core/core-rest-pipeline/src/pipelineRequest.ts | 6 ------ sdk/core/core-rest-pipeline/src/xhrHttpClient.ts | 13 +++++++++++-- .../test/browser/xhrHttpClient.spec.ts | 2 +- .../test/node/nodeHttpClient.spec.ts | 2 +- 7 files changed, 19 insertions(+), 18 deletions(-) diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index 00a523abc1ab..5a98fd4448ab 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -199,7 +199,6 @@ export interface PipelineRequest { onUploadProgress?: (progress: TransferProgressEvent) => void; proxySettings?: ProxySettings; requestId: string; - responseAsStream?: boolean; streamResponseStatusCodes?: Set; timeout: number; tracingOptions?: OperationTracingOptions; @@ -220,7 +219,6 @@ export interface PipelineRequestOptions { onUploadProgress?: (progress: TransferProgressEvent) => void; proxySettings?: ProxySettings; requestId?: string; - responseAsStream?: boolean; streamResponseStatusCodes?: Set; timeout?: number; tracingOptions?: OperationTracingOptions; diff --git a/sdk/core/core-rest-pipeline/src/interfaces.ts b/sdk/core/core-rest-pipeline/src/interfaces.ts index ed2acdb13158..ab27c078c687 100644 --- a/sdk/core/core-rest-pipeline/src/interfaces.ts +++ b/sdk/core/core-rest-pipeline/src/interfaces.ts @@ -138,14 +138,10 @@ export interface PipelineRequest { /** * A list of response status codes whose corresponding PipelineResponse body should be treated as a stream. + * When streamResponseStatusCodes contains the value Number.POSITIVE_INFINITY any status would be treated as a stream. */ streamResponseStatusCodes?: Set; - /** - * If set, body will be returned as a raw stream. - */ - responseAsStream?: boolean; - /** * Proxy configuration. */ diff --git a/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts b/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts index 64eef6eaa9de..4e2346fa65ee 100644 --- a/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts +++ b/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts @@ -151,7 +151,11 @@ class NodeHttpClient implements HttpClient { responseStream = downloadReportStream; } - if (request.responseAsStream || request.streamResponseStatusCodes?.has(response.status)) { + if ( + // Value of POSITIVE_INFINITY in streamResponseStatusCodes is considered as any status code + request.streamResponseStatusCodes?.has(Number.POSITIVE_INFINITY) || + request.streamResponseStatusCodes?.has(response.status) + ) { response.readableStreamBody = responseStream; } else { response.bodyAsText = await streamToText(responseStream); diff --git a/sdk/core/core-rest-pipeline/src/pipelineRequest.ts b/sdk/core/core-rest-pipeline/src/pipelineRequest.ts index c249454f2177..3f4994164559 100644 --- a/sdk/core/core-rest-pipeline/src/pipelineRequest.ts +++ b/sdk/core/core-rest-pipeline/src/pipelineRequest.ts @@ -68,11 +68,6 @@ export interface PipelineRequestOptions { */ streamResponseStatusCodes?: Set; - /** - * If set, body will be returned as a raw stream. - */ - responseAsStream?: boolean; - /** * Proxy configuration. */ @@ -135,7 +130,6 @@ class PipelineRequestImpl implements PipelineRequest { this.disableKeepAlive = options.disableKeepAlive ?? false; this.proxySettings = options.proxySettings; this.streamResponseStatusCodes = options.streamResponseStatusCodes; - this.responseAsStream = options.responseAsStream; this.withCredentials = options.withCredentials ?? false; this.abortSignal = options.abortSignal; this.tracingOptions = options.tracingOptions; diff --git a/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts b/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts index 3ea7be248073..bfd005e6a6bf 100644 --- a/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts +++ b/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts @@ -67,8 +67,13 @@ class XhrHttpClient implements HttpClient { for (const [name, value] of request.headers) { xhr.setRequestHeader(name, value); } + xhr.responseType = - request.responseAsStream || request.streamResponseStatusCodes?.size ? "blob" : "text"; + // Value of POSITIVE_INFINITY in streamResponseStatusCodes is considered as any status code + request.streamResponseStatusCodes?.has(Number.POSITIVE_INFINITY) || + request.streamResponseStatusCodes?.size + ? "blob" + : "text"; if (isReadableStream(request.body)) { throw new Error("Node streams are not supported in browser environment."); @@ -106,7 +111,11 @@ function handleBlobResponse( xhr.addEventListener("readystatechange", () => { // Resolve as soon as headers are loaded if (xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED) { - if (request.responseAsStream || request.streamResponseStatusCodes?.has(xhr.status)) { + if ( + // Value of POSITIVE_INFINITY in streamResponseStatusCodes is considered as any status code + request.streamResponseStatusCodes?.has(Number.POSITIVE_INFINITY) || + request.streamResponseStatusCodes?.has(xhr.status) + ) { const blobBody = new Promise((resolve, reject) => { xhr.addEventListener("load", () => { resolve(xhr.response); diff --git a/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts b/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts index 15706eb0a01f..dd1080f2e93b 100644 --- a/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts +++ b/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts @@ -157,7 +157,7 @@ describe("XhrHttpClient", function() { const client = createDefaultHttpClient(); const request = createPipelineRequest({ url: "https://example.com", - responseAsStream: true + streamResponseStatusCodes: new Set([Number.POSITIVE_INFINITY]) }); const promise = client.sendRequest(request); assert.equal(requests.length, 1); diff --git a/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts b/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts index 48264da5e236..96159a1f9d69 100644 --- a/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts +++ b/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts @@ -200,7 +200,7 @@ describe("NodeHttpClient", function() { stubbedHttpsRequest.returns(clientRequest); const request = createPipelineRequest({ url: "https://example.com", - responseAsStream: true + streamResponseStatusCodes: new Set([Number.POSITIVE_INFINITY]) }); const promise = client.sendRequest(request); stubbedHttpsRequest.yield(createResponse(200, "body")); From 3ef99ae1e519d330c6b40a34f26e8e5455681890 Mon Sep 17 00:00:00 2001 From: Jose Manuel Heredia Hidalgo Date: Tue, 2 Nov 2021 13:29:48 -0700 Subject: [PATCH 3/4] Remove unneeded property in client request and address comments --- sdk/core/core-rest-pipeline/src/pipelineRequest.ts | 1 - sdk/core/core-rest-pipeline/src/xhrHttpClient.ts | 7 +------ .../core-rest-pipeline/test/browser/xhrHttpClient.spec.ts | 6 +++--- .../core-rest-pipeline/test/node/nodeHttpClient.spec.ts | 4 ++-- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/pipelineRequest.ts b/sdk/core/core-rest-pipeline/src/pipelineRequest.ts index 3f4994164559..1828818f5f96 100644 --- a/sdk/core/core-rest-pipeline/src/pipelineRequest.ts +++ b/sdk/core/core-rest-pipeline/src/pipelineRequest.ts @@ -109,7 +109,6 @@ class PipelineRequestImpl implements PipelineRequest { public body?: RequestBodyType; public formData?: FormDataMap; public streamResponseStatusCodes?: Set; - public responseAsStream?: boolean; public proxySettings?: ProxySettings; public disableKeepAlive: boolean; diff --git a/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts b/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts index bfd005e6a6bf..090e948afa9b 100644 --- a/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts +++ b/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts @@ -68,12 +68,7 @@ class XhrHttpClient implements HttpClient { xhr.setRequestHeader(name, value); } - xhr.responseType = - // Value of POSITIVE_INFINITY in streamResponseStatusCodes is considered as any status code - request.streamResponseStatusCodes?.has(Number.POSITIVE_INFINITY) || - request.streamResponseStatusCodes?.size - ? "blob" - : "text"; + xhr.responseType = request.streamResponseStatusCodes?.size ? "blob" : "text"; if (isReadableStream(request.body)) { throw new Error("Node streams are not supported in browser environment."); diff --git a/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts b/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts index dd1080f2e93b..6aa5f9be526a 100644 --- a/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts +++ b/sdk/core/core-rest-pipeline/test/browser/xhrHttpClient.spec.ts @@ -153,7 +153,7 @@ describe("XhrHttpClient", function() { assert.ok(response.blobBody, "Expect streaming body"); }); - it("should stream response body on responseAsStream", async function() { + it("should stream response body on any status code", async function() { const client = createDefaultHttpClient(); const request = createPipelineRequest({ url: "https://example.com", @@ -161,9 +161,9 @@ describe("XhrHttpClient", function() { }); const promise = client.sendRequest(request); assert.equal(requests.length, 1); - requests[0].respond(200, {}, "body"); + requests[0].respond(201, {}, "body"); const response = await promise; - assert.strictEqual(response.status, 200); + assert.strictEqual(response.status, 201); assert.equal(response.bodyAsText, undefined); assert.ok(response.blobBody, "Expect streaming body"); }); diff --git a/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts b/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts index 96159a1f9d69..e44933a125d6 100644 --- a/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts +++ b/sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts @@ -194,7 +194,7 @@ describe("NodeHttpClient", function() { assert.ok(response.readableStreamBody); }); - it("should stream response body on responseAsStream", async function() { + it("should stream response body on any status code", async function() { const client = createDefaultHttpClient(); const clientRequest = createRequest(); stubbedHttpsRequest.returns(clientRequest); @@ -203,7 +203,7 @@ describe("NodeHttpClient", function() { streamResponseStatusCodes: new Set([Number.POSITIVE_INFINITY]) }); const promise = client.sendRequest(request); - stubbedHttpsRequest.yield(createResponse(200, "body")); + stubbedHttpsRequest.yield(createResponse(201, "body")); const response = await promise; assert.equal(response.bodyAsText, undefined); assert.ok(response.readableStreamBody); From e2e26fcfb36205466b9a813535ff949c9a572586 Mon Sep 17 00:00:00 2001 From: Jose Manuel Heredia Hidalgo Date: Wed, 3 Nov 2021 10:47:03 -0700 Subject: [PATCH 4/4] Update changelog --- sdk/core/core-rest-pipeline/CHANGELOG.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sdk/core/core-rest-pipeline/CHANGELOG.md b/sdk/core/core-rest-pipeline/CHANGELOG.md index d1b5e4f640b6..924c3554d170 100644 --- a/sdk/core/core-rest-pipeline/CHANGELOG.md +++ b/sdk/core/core-rest-pipeline/CHANGELOG.md @@ -1,15 +1,11 @@ # Release History -## 1.3.2 (Unreleased) - -### Features Added - -### Breaking Changes - -### Bugs Fixed +## 1.3.2 (2021-11-4) ### Other Changes +- Allow specifying any status response to get a raw stream as response content. [#18492](https://github.com/Azure/azure-sdk-for-js/pull/18492) + ## 1.3.1 (2021-09-30) ### Bugs Fixed