diff --git a/.changeset/curvy-peaches-deliver.md b/.changeset/curvy-peaches-deliver.md new file mode 100644 index 00000000000..1112887cc1b --- /dev/null +++ b/.changeset/curvy-peaches-deliver.md @@ -0,0 +1,5 @@ +--- +"@firebase/storage": patch +--- + +Do not throw from `connection.send` to enable retries in Node.js. diff --git a/packages/storage/src/implementation/connection.ts b/packages/storage/src/implementation/connection.ts index 94f46a5a4f5..850c5e6e265 100644 --- a/packages/storage/src/implementation/connection.ts +++ b/packages/storage/src/implementation/connection.ts @@ -23,6 +23,10 @@ export type Headers = Record; * goog.net.XhrIo-like interface. */ export interface Connection { + /** + * This method should never reject. In case of encountering an error, set an error code internally which can be accessed + * by calling getErrorCode() by callers. + */ send( url: string, method: string, diff --git a/packages/storage/src/implementation/request.ts b/packages/storage/src/implementation/request.ts index 79d09009f75..5c992ba6f1c 100644 --- a/packages/storage/src/implementation/request.ts +++ b/packages/storage/src/implementation/request.ts @@ -106,6 +106,7 @@ class NetworkRequest implements Request { connection.addUploadProgressListener(progressListener); } + // connection.send() never rejects, so we don't need to have a error handler or use catch on the returned promise. // eslint-disable-next-line @typescript-eslint/no-floating-promises connection .send(this.url_, this.method_, this.body_, this.headers_) diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index 1d4633fd77b..cac38e016c7 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -17,10 +17,7 @@ import { ErrorCode, Connection } from '../../implementation/connection'; import { internalError } from '../../implementation/error'; -import nodeFetch from 'node-fetch'; - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const fetch: typeof window.fetch = nodeFetch as any; +import nodeFetch, { FetchError } from 'node-fetch'; /** * Network layer that works in Node. @@ -34,6 +31,8 @@ export class FetchConnection implements Connection { private body_: string | undefined; private headers_: Headers | undefined; private sent_: boolean = false; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private fetch_: typeof window.fetch = nodeFetch as any; constructor() { this.errorCode_ = ErrorCode.NO_ERROR; @@ -50,7 +49,7 @@ export class FetchConnection implements Connection { } this.sent_ = true; - return fetch(url, { + return this.fetch_(url, { method, headers: headers || {}, body @@ -58,10 +57,13 @@ export class FetchConnection implements Connection { .then(resp => { this.headers_ = resp.headers; this.statusCode_ = resp.status; - return resp.text(); - }) - .then(body => { - this.body_ = body; + return resp.text().then(body => { + this.body_ = body; + }); + }, (_err: FetchError) => { + this.errorCode_ = ErrorCode.NETWORK_ERROR; + // emulate XHR which sets status to 0 when encountering a network error + this.statusCode_ = 0; }); } diff --git a/packages/storage/test/browser/connection.test.ts b/packages/storage/test/browser/connection.test.ts new file mode 100644 index 00000000000..13e8cf27cbe --- /dev/null +++ b/packages/storage/test/browser/connection.test.ts @@ -0,0 +1,17 @@ +import { expect } from "chai"; +import { SinonFakeXMLHttpRequest, useFakeXMLHttpRequest } from "sinon"; +import { ErrorCode } from "../../src/implementation/connection"; +import { XhrConnection } from "../../src/platform/browser/connection"; + +describe('Connections', () => { + it('XhrConnection.send() should not reject on network errors', async () => { + const fakeXHR = useFakeXMLHttpRequest(); + const connection = new XhrConnection(); + const sendPromise = connection.send('testurl', 'GET'); + // simulate a network error + ((connection as any).xhr_ as SinonFakeXMLHttpRequest).error(); + await sendPromise; + expect(connection.getErrorCode()).to.equal(ErrorCode.NETWORK_ERROR); + fakeXHR.restore(); + }); +}); diff --git a/packages/storage/test/unit/connection.test.ts b/packages/storage/test/unit/connection.test.ts new file mode 100644 index 00000000000..b79fa459a7b --- /dev/null +++ b/packages/storage/test/unit/connection.test.ts @@ -0,0 +1,15 @@ +import { stub } from 'sinon'; +import { expect } from 'chai'; +import { ErrorCode } from '../../src/implementation/connection'; +import { FetchConnection } from '../../src/platform/node/connection'; + +describe('Connections', () => { + it('FetchConnection.send() should not reject on network errors', async () => { + const connection = new FetchConnection(); + + // need the casting here because fetch_ is a private member + stub(connection as any, 'fetch_').rejects(); + await connection.send('testurl', 'GET'); + expect(connection.getErrorCode()).to.equal(ErrorCode.NETWORK_ERROR); + }); +});