Skip to content

Commit c1c06a1

Browse files
committed
Store decodeStream separately in RestClient class
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
1 parent 6451c2c commit c1c06a1

4 files changed

Lines changed: 34 additions & 44 deletions

File tree

packages/imperative/src/rest/__tests__/client/AbstractRestClient.unit.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -916,9 +916,10 @@ describe("AbstractRestClient tests", () => {
916916

917917
const fakeResponseStream: any = {
918918
write: jest.fn(),
919-
on: jest.fn(),
920-
end: jest.fn((cb: any) => cb()),
921-
writableFinished: true
919+
on: jest.fn((eventName: string, callback: any) => {
920+
if (eventName === "finish") callback();
921+
}),
922+
end: jest.fn(),
922923
};
923924
const fakeRequestStream: any = {
924925
on: jest.fn((_eventName: string, _callback: any) => {
@@ -1455,7 +1456,7 @@ describe("AbstractRestClient tests", () => {
14551456
}
14561457

14571458
expect(caughtError instanceof ImperativeError).toBe(true);
1458-
expect(caughtError.message).toMatchSnapshot();
1459+
expect(caughtError.message).toContain("Failed to decompress response buffer");
14591460
});
14601461

14611462
it("should error when decompressing invalid gzip stream", async () => {
@@ -1494,7 +1495,7 @@ describe("AbstractRestClient tests", () => {
14941495
}
14951496

14961497
expect(caughtError instanceof ImperativeError).toBe(true);
1497-
expect(caughtError.message).toMatchSnapshot();
1498+
expect(caughtError.message).toContain("Failed to decompress response stream");
14981499
});
14991500

15001501
it("should error when decompressing truncated gzip stream with text content", async () => {
@@ -1533,7 +1534,7 @@ describe("AbstractRestClient tests", () => {
15331534
}
15341535

15351536
expect(caughtError instanceof ImperativeError).toBe(true);
1536-
expect(caughtError.message).toMatchSnapshot();
1537+
expect(caughtError.message).toContain("Failed to decompress response stream");
15371538
});
15381539

15391540
it("should decompress error message for streamed request", async () => {

packages/imperative/src/rest/__tests__/client/__snapshots__/AbstractRestClient.unit.test.ts.snap

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`AbstractRestClient tests content encoding should error when decompressing invalid gzip buffer 1`] = `"Failed to decompress response buffer with content encoding type gzip"`;
4-
5-
exports[`AbstractRestClient tests content encoding should error when decompressing invalid gzip stream 1`] = `"Failed to decompress response stream with content encoding type gzip"`;
6-
7-
exports[`AbstractRestClient tests content encoding should error when decompressing truncated gzip stream with text content 1`] = `"Failed to decompress response stream with content encoding type gzip"`;
8-
93
exports[`AbstractRestClient tests should create buildOptions according to input parameter options 1 1`] = `
104
Object {
115
"headers": Object {

packages/imperative/src/rest/src/client/AbstractRestClient.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ export abstract class AbstractRestClient {
183183
*/
184184
protected mResponseStream: Writable;
185185

186+
/**
187+
* If content encoding is used, this is the decompression stream that we write to.
188+
* It pipes through transforms to mResponseStream.
189+
* @private
190+
* @type {Writable}
191+
* @memberof AbstractRestClient
192+
*/
193+
protected mDecodeStream: Writable;
194+
186195
/**
187196
* stream for outgoing request data to the server
188197
* @private
@@ -815,7 +824,7 @@ export abstract class AbstractRestClient {
815824
if (this.mContentEncoding != null) {
816825
this.log.debug("Adding decompression transform to response stream");
817826
try {
818-
this.mResponseStream = CompressionUtils.decompressStream(this.mResponseStream, this.mContentEncoding,
827+
this.mDecodeStream = CompressionUtils.decompressStream(this.mResponseStream, this.mContentEncoding,
819828
this.mNormalizeResponseNewlines);
820829
} catch (err) {
821830
this.mReject(err);
@@ -878,7 +887,7 @@ export abstract class AbstractRestClient {
878887
}
879888
}
880889
// write the chunk to the response stream if requested
881-
this.mResponseStream.write(respData);
890+
(this.mDecodeStream ?? this.mResponseStream).write(respData);
882891
this.lastByteReceived = respData[respData.byteLength - 1];
883892
}
884893
}
@@ -924,7 +933,8 @@ export abstract class AbstractRestClient {
924933
};
925934
if (this.mResponseStream != null) {
926935
this.log.debug("Ending response stream");
927-
this.mResponseStream.end(requestEnd);
936+
this.mResponseStream.on("finish", requestEnd);
937+
(this.mDecodeStream ?? this.mResponseStream).end();
928938
} else {
929939
requestEnd();
930940
}

packages/imperative/src/rest/src/client/CompressionUtils.ts

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*/
1111

12-
import { Duplex, PassThrough, pipeline, Transform, Writable } from "stream";
12+
import { Transform, Writable } from "stream";
1313
import * as zlib from "zlib";
1414
import { ImperativeError } from "../../../error";
1515
import { IO } from "../../../io";
@@ -51,46 +51,31 @@ export class CompressionUtils {
5151
* @param normalizeNewLines Specifies if line endings should be converted
5252
* @throws {ImperativeError}
5353
*/
54-
public static decompressStream(responseStream: Writable, encoding: ContentEncoding, normalizeNewLines?: boolean): Duplex {
54+
public static decompressStream(responseStream: Writable, encoding: ContentEncoding, normalizeNewLines?: boolean): Writable {
5555
if (!Headers.CONTENT_ENCODING_TYPES.includes(encoding)) {
5656
throw new ImperativeError({ msg: `Unsupported content encoding type ${encoding}` });
5757
}
5858

5959
try {
60-
const inputStream = new PassThrough();
60+
// First transform handles decompression
61+
const transforms = [this.zlibTransform(encoding, !normalizeNewLines)];
6162

62-
// Build the transform pipeline
63-
const transforms: Transform[] = [this.zlibTransform(encoding, !normalizeNewLines)];
63+
// Second transform is optional and processes line endings
6464
if (normalizeNewLines) {
6565
transforms.push(this.newLinesTransform());
6666
}
6767

68-
// Wrapper stream that waits for pipeline completion
69-
let finalCallback: ((error?: Error | null) => void) | undefined;
70-
const wrapper = new Duplex({
71-
write(chunk, encoding, callback) {
72-
if (!inputStream.write(chunk, encoding)) {
73-
inputStream.once("drain", callback);
74-
} else {
75-
callback();
76-
}
77-
},
78-
final(callback) {
79-
finalCallback = callback;
80-
inputStream.end();
81-
},
82-
read() { /* write-only from caller's perspective */ }
83-
});
84-
85-
// Connect pipeline and notify wrapper when complete
86-
pipeline([inputStream, ...transforms, responseStream], (err) => {
87-
if (err) {
68+
// Chain transforms and response stream together
69+
for (const [i, stream] of transforms.entries()) {
70+
const next = transforms[i + 1] || responseStream;
71+
stream.pipe(next);
72+
stream.on("error", (err) => {
8873
responseStream.emit("error", this.decompressError(err, "stream", encoding));
89-
}
90-
finalCallback?.(err);
91-
});
74+
});
75+
}
9276

93-
return wrapper;
77+
// Return first stream in chain
78+
return transforms[0];
9479
} catch (err) {
9580
throw this.decompressError(err, "stream", encoding);
9681
}

0 commit comments

Comments
 (0)