Skip to content

Commit 7462b07

Browse files
kuheAllanZhengYP
andauthored
feat(xhr-http-handler): add XMLHttpRequest http handler to use with lib-storage Upload (#3798)
* feat(xhr-http-handler): add xhr handler * feat(xhr-http-handler): remove debug output * feat(xhr-http-handler): cleanup for PR * feat(xhr-http-handler): add error handler * accept suggestion in packages/xhr-http-handler/src/request-timeout.ts Co-authored-by: AllanZhengYP <[email protected]> * feat(xhr-http-handler): add streaming text handling * feat(xhr-http-handler): update unit tests * fix(xhr-http-handler): readme update * fix(xhr-http-handler): readme update * fix(xhr-http-handler): readme update * fix(fetch-http-handler): modify config awaiter to be consistent with other http handlers * doc(xhr-http-handler): improve documentation for download progress events Co-authored-by: AllanZhengYP <[email protected]>
1 parent 1702c43 commit 7462b07

18 files changed

+1049
-11
lines changed

Diff for: lib/lib-storage/src/Upload.spec.ts

+43-6
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,19 @@ const endpointMock = jest.fn().mockResolvedValue({
2929
query: undefined,
3030
});
3131

32+
import { EventEmitter, Readable } from "stream";
33+
34+
const mockAddListener = jest.fn();
35+
const mockRemoveListener = jest.fn();
36+
const requestHandlerMock = (() => {
37+
const mock = {
38+
on: mockAddListener,
39+
off: mockRemoveListener,
40+
};
41+
Object.setPrototypeOf(mock, EventEmitter.prototype);
42+
return mock;
43+
})();
44+
3245
jest.mock("@aws-sdk/client-s3", () => ({
3346
...(jest.requireActual("@aws-sdk/client-s3") as {}),
3447
S3: jest.fn().mockReturnValue({
@@ -41,6 +54,7 @@ jest.mock("@aws-sdk/client-s3", () => ({
4154
send: sendMock,
4255
config: {
4356
endpoint: endpointMock,
57+
requestHandler: requestHandlerMock,
4458
},
4559
}),
4660
CreateMultipartUploadCommand: createMultipartMock,
@@ -50,9 +64,8 @@ jest.mock("@aws-sdk/client-s3", () => ({
5064
PutObjectCommand: putObjectMock,
5165
}));
5266

53-
import { CompleteMultipartUploadCommandOutput, S3 } from "@aws-sdk/client-s3";
67+
import { CompleteMultipartUploadCommandOutput, S3, S3Client } from "@aws-sdk/client-s3";
5468
import { createHash } from "crypto";
55-
import { Readable } from "stream";
5669

5770
import { Progress, Upload } from "./index";
5871

@@ -498,7 +511,7 @@ describe(Upload.name, () => {
498511
client: new S3({}),
499512
});
500513

501-
const received = [];
514+
const received: Progress[] = [];
502515
upload.on("httpUploadProgress", (progress: Progress) => {
503516
received.push(progress);
504517
});
@@ -534,7 +547,7 @@ describe(Upload.name, () => {
534547
client: new S3({}),
535548
});
536549

537-
const received = [];
550+
const received: Progress[] = [];
538551
upload.on("httpUploadProgress", (progress: Progress) => {
539552
received.push(progress);
540553
});
@@ -564,7 +577,7 @@ describe(Upload.name, () => {
564577
client: new S3({}),
565578
});
566579

567-
const received = [];
580+
const received: Progress[] = [];
568581
upload.on("httpUploadProgress", (progress: Progress) => {
569582
received.push(progress);
570583
});
@@ -587,7 +600,7 @@ describe(Upload.name, () => {
587600
client: new S3({}),
588601
});
589602

590-
const received = [];
603+
const received: Progress[] = [];
591604
upload.on("httpUploadProgress", (progress: Progress) => {
592605
received.push(progress);
593606
});
@@ -601,4 +614,28 @@ describe(Upload.name, () => {
601614
});
602615
expect(received.length).toBe(1);
603616
});
617+
618+
it("listens to the requestHandler for updates if it is an EventEmitter", async () => {
619+
const partSize = 1024 * 1024 * 5;
620+
const largeBuffer = Buffer.from("#".repeat(partSize + 10));
621+
const streamBody = Readable.from(
622+
(function* () {
623+
yield largeBuffer;
624+
})()
625+
);
626+
const actionParams = { ...params, Body: streamBody };
627+
const upload = new Upload({
628+
params: actionParams,
629+
client: new S3Client({}),
630+
});
631+
632+
const received: Progress[] = [];
633+
upload.on("httpUploadProgress", (progress: Progress) => {
634+
received.push(progress);
635+
});
636+
await upload.done();
637+
expect(received.length).toBe(2);
638+
expect(mockAddListener).toHaveBeenCalledWith("xhr.upload.progress", expect.any(Function));
639+
expect(mockRemoveListener).toHaveBeenCalledWith("xhr.upload.progress", expect.any(Function));
640+
});
604641
});

Diff for: lib/lib-storage/src/Upload.ts

+73-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
Tag,
1414
UploadPartCommand,
1515
} from "@aws-sdk/client-s3";
16+
import { HttpRequest } from "@aws-sdk/protocol-http";
1617
import { extendedEncodeURIComponent } from "@aws-sdk/smithy-client";
1718
import { EventEmitter } from "events";
1819

@@ -99,11 +100,35 @@ export class Upload extends EventEmitter {
99100
private async __uploadUsingPut(dataPart: RawDataPart): Promise<void> {
100101
this.isMultiPart = false;
101102
const params = { ...this.params, Body: dataPart.data };
103+
104+
const requestHandler = this.client.config.requestHandler;
105+
const eventEmitter: EventEmitter | null = requestHandler instanceof EventEmitter ? requestHandler : null;
106+
const uploadEventListener = (event: ProgressEvent) => {
107+
this.bytesUploadedSoFar = event.loaded;
108+
this.totalBytes = event.total;
109+
this.__notifyProgress({
110+
loaded: this.bytesUploadedSoFar,
111+
total: this.totalBytes,
112+
part: dataPart.partNumber,
113+
Key: this.params.Key,
114+
Bucket: this.params.Bucket,
115+
});
116+
};
117+
118+
if (eventEmitter !== null) {
119+
// The requestHandler is the xhr-http-handler.
120+
eventEmitter.on("xhr.upload.progress", uploadEventListener);
121+
}
122+
102123
const [putResult, endpoint] = await Promise.all([
103124
this.client.send(new PutObjectCommand(params)),
104125
this.client.config.endpoint(),
105126
]);
106127

128+
if (eventEmitter !== null) {
129+
eventEmitter.off("xhr.upload.progress", uploadEventListener);
130+
}
131+
107132
const locationKey = this.params
108133
.Key!.split("/")
109134
.map((segment) => extendedEncodeURIComponent(segment))
@@ -121,6 +146,7 @@ export class Upload extends EventEmitter {
121146
Location,
122147
};
123148
const totalSize = byteLength(dataPart.data);
149+
124150
this.__notifyProgress({
125151
loaded: totalSize,
126152
total: totalSize,
@@ -164,6 +190,39 @@ export class Upload extends EventEmitter {
164190
}
165191
}
166192

193+
const partSize: number = byteLength(dataPart.data) || 0;
194+
195+
const requestHandler = this.client.config.requestHandler;
196+
const eventEmitter: EventEmitter | null = requestHandler instanceof EventEmitter ? requestHandler : null;
197+
198+
let lastSeenBytes = 0;
199+
const uploadEventListener = (event: ProgressEvent, request: HttpRequest) => {
200+
const requestPartSize = Number(request.query["partNumber"]) || -1;
201+
202+
if (requestPartSize !== dataPart.partNumber) {
203+
// ignored, because the emitted event is not for this part.
204+
return;
205+
}
206+
207+
if (event.total && partSize) {
208+
this.bytesUploadedSoFar += event.loaded - lastSeenBytes;
209+
lastSeenBytes = event.loaded;
210+
}
211+
212+
this.__notifyProgress({
213+
loaded: this.bytesUploadedSoFar,
214+
total: this.totalBytes,
215+
part: dataPart.partNumber,
216+
Key: this.params.Key,
217+
Bucket: this.params.Bucket,
218+
});
219+
};
220+
221+
if (eventEmitter !== null) {
222+
// The requestHandler is the xhr-http-handler.
223+
eventEmitter.on("xhr.upload.progress", uploadEventListener);
224+
}
225+
167226
const partResult = await this.client.send(
168227
new UploadPartCommand({
169228
...this.params,
@@ -173,10 +232,20 @@ export class Upload extends EventEmitter {
173232
})
174233
);
175234

235+
if (eventEmitter !== null) {
236+
eventEmitter.off("xhr.upload.progress", uploadEventListener);
237+
}
238+
176239
if (this.abortController.signal.aborted) {
177240
return;
178241
}
179242

243+
if (!partResult.ETag) {
244+
throw new Error(
245+
`Part ${dataPart.partNumber} is missing ETag in UploadPart response. Missing Bucket CORS configuration for ETag header?`
246+
);
247+
}
248+
180249
this.uploadedParts.push({
181250
PartNumber: dataPart.partNumber,
182251
ETag: partResult.ETag,
@@ -186,7 +255,10 @@ export class Upload extends EventEmitter {
186255
...(partResult.ChecksumSHA256 && { ChecksumSHA256: partResult.ChecksumSHA256 }),
187256
});
188257

189-
this.bytesUploadedSoFar += byteLength(dataPart.data);
258+
if (eventEmitter === null) {
259+
this.bytesUploadedSoFar += partSize;
260+
}
261+
190262
this.__notifyProgress({
191263
loaded: this.bytesUploadedSoFar,
192264
total: this.totalBytes,

Diff for: packages/fetch-http-handler/src/fetch-http-handler.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ type FetchHttpHandlerConfig = FetchHttpHandlerOptions;
2121

2222
export class FetchHttpHandler implements HttpHandler {
2323
private config?: FetchHttpHandlerConfig;
24-
private readonly configProvider?: Provider<FetchHttpHandlerConfig>;
24+
private readonly configProvider: Promise<FetchHttpHandlerConfig>;
2525

2626
constructor(options?: FetchHttpHandlerOptions | Provider<FetchHttpHandlerOptions | undefined>) {
2727
if (typeof options === "function") {
28-
this.configProvider = async () => (await options()) || {};
28+
this.configProvider = options().then((opts) => opts || {});
2929
} else {
3030
this.config = options ?? {};
31+
this.configProvider = Promise.resolve(this.config);
3132
}
3233
}
3334

@@ -36,8 +37,8 @@ export class FetchHttpHandler implements HttpHandler {
3637
}
3738

3839
async handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
39-
if (!this.config && this.configProvider) {
40-
this.config = await this.configProvider();
40+
if (!this.config) {
41+
this.config = await this.configProvider;
4142
}
4243
const requestTimeoutInMs = this.config!.requestTimeout;
4344

Diff for: packages/xhr-http-handler/.gitignore

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/node_modules/
2+
/build/
3+
/coverage/
4+
/docs/
5+
*.tsbuildinfo
6+
*.tgz
7+
*.log
8+
package-lock.json

Diff for: packages/xhr-http-handler/CHANGELOG.md

Whitespace-only changes.

0 commit comments

Comments
 (0)