From 8c4d246d6d0cc0b22510f112d2d5176ed8285298 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Wed, 14 Oct 2020 12:13:47 -0700 Subject: [PATCH 1/5] Fix storage-types --- packages/storage-types/index.d.ts | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/storage-types/index.d.ts b/packages/storage-types/index.d.ts index 902461581cc..db5efc778cf 100644 --- a/packages/storage-types/index.d.ts +++ b/packages/storage-types/index.d.ts @@ -16,7 +16,6 @@ */ import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types'; -import { Observer, Unsubscribe } from '@firebase/util'; export interface FullMetadata extends UploadMetadata { bucket: string; @@ -48,7 +47,7 @@ export interface Reference { metadata?: UploadMetadata ): UploadTask; root: Reference; - storage: Storage; + storage: FirebaseStorage; toString(): string; updateMetadata(metadata: SettableMetadata): Promise; listAll(): Promise; @@ -85,16 +84,33 @@ export interface UploadMetadata extends SettableMetadata { md5Hash?: string | null; } +export interface FirebaseStorageError { + name: string; + code: string; + message: string; + serverResponse: null | string; +} + +export type NextFn = (value: T) => void; +export type ErrorFn = (error: FirebaseStorageError) => void; +export type CompleteFn = () => void; +export type Unsubscribe = () => void; +export interface StorageObserver { + next?: NextFn | null; + error?: ErrorFn | null; + complete?: CompleteFn | null; +} + export interface UploadTask { cancel(): boolean; - catch(onRejected: (a: Error) => any): Promise; + catch(onRejected: (a: FirebaseStorageError) => any): Promise; on( event: TaskEvent, nextOrObserver?: - | Partial> + | Partial> | null - | ((a: UploadTaskSnapshot) => any), - error?: ((a: Error) => any) | null, + | ((a: UploadTaskSnapshot) => unknown), + error?: ((a: FirebaseStorageError) => any) | null, complete?: Unsubscribe | null ): Function; pause(): boolean; @@ -102,7 +118,7 @@ export interface UploadTask { snapshot: UploadTaskSnapshot; then( onFulfilled?: ((a: UploadTaskSnapshot) => any) | null, - onRejected?: ((a: Error) => any) | null + onRejected?: ((a: FirebaseStorageError) => any) | null ): Promise; } From 2a8b2aaf1b846c85e414ab280c5fcd226cfd7bdb Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Thu, 15 Oct 2020 09:51:02 -0700 Subject: [PATCH 2/5] Add changes to top-level index.d.ts --- packages/firebase/index.d.ts | 32 ++++++++++++++++++++++++------- packages/storage-types/index.d.ts | 19 ++++++++---------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index c0d71742d59..0dee698efce 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -7617,6 +7617,22 @@ declare namespace firebase.storage { md5Hash?: string | null; } + /** + * An error returned by the Firebase Storage SDK. + */ + interface FirebaseStorageError { + name: string; + code: string; + message: string; + serverResponse: null | string; + } + + interface StorageObserver { + next?: NextFn | null; + error?: (error: FirebaseStorageError) => void | null; + complete?: CompleteFn | null; + } + /** * Represents the process of uploading an object. Allows you to monitor and * manage the upload. @@ -7630,7 +7646,7 @@ declare namespace firebase.storage { /** * Equivalent to calling `then(null, onRejected)`. */ - catch(onRejected: (a: Error) => any): Promise; + catch(onRejected: (error: FirebaseStorageError) => any): Promise; /** * Listens for events on this task. * @@ -7730,7 +7746,7 @@ declare namespace firebase.storage { * The `next` function, which gets called for each item in * the event stream, or an observer object with some or all of these three * properties (`next`, `error`, `complete`). - * @param error A function that gets called with an Error + * @param error A function that gets called with a `FirebaseStorageError` * if the event stream ends due to an error. * @param complete A function that gets called if the * event stream ends normally. @@ -7743,10 +7759,10 @@ declare namespace firebase.storage { on( event: firebase.storage.TaskEvent, nextOrObserver?: - | Partial> + | StorageObserver | null - | ((a: UploadTaskSnapshot) => any), - error?: ((a: Error) => any) | null, + | ((snapshot: UploadTaskSnapshot) => any), + error?: ((error: FirebaseStorageError) => any) | null, complete?: firebase.Unsubscribe | null ): Function; /** @@ -7771,8 +7787,10 @@ declare namespace firebase.storage { * @param onRejected The rejection callback. */ then( - onFulfilled?: ((a: firebase.storage.UploadTaskSnapshot) => any) | null, - onRejected?: ((a: Error) => any) | null + onFulfilled?: + | ((snapshot: firebase.storage.UploadTaskSnapshot) => any) + | null, + onRejected?: ((error: FirebaseStorageError) => any) | null ): Promise; } diff --git a/packages/storage-types/index.d.ts b/packages/storage-types/index.d.ts index db5efc778cf..c8e5201807d 100644 --- a/packages/storage-types/index.d.ts +++ b/packages/storage-types/index.d.ts @@ -15,7 +15,8 @@ * limitations under the License. */ -import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types'; +import { FirebaseApp } from '@firebase/app-types'; +import { CompleteFn, NextFn, Unsubscribe } from '@firebase/util'; export interface FullMetadata extends UploadMetadata { bucket: string; @@ -91,25 +92,21 @@ export interface FirebaseStorageError { serverResponse: null | string; } -export type NextFn = (value: T) => void; -export type ErrorFn = (error: FirebaseStorageError) => void; -export type CompleteFn = () => void; -export type Unsubscribe = () => void; export interface StorageObserver { next?: NextFn | null; - error?: ErrorFn | null; + error?: (error: FirebaseStorageError) => void | null; complete?: CompleteFn | null; } export interface UploadTask { cancel(): boolean; - catch(onRejected: (a: FirebaseStorageError) => any): Promise; + catch(onRejected: (error: FirebaseStorageError) => any): Promise; on( event: TaskEvent, nextOrObserver?: - | Partial> + | StorageObserver | null - | ((a: UploadTaskSnapshot) => unknown), + | ((snapshot: UploadTaskSnapshot) => any), error?: ((a: FirebaseStorageError) => any) | null, complete?: Unsubscribe | null ): Function; @@ -117,8 +114,8 @@ export interface UploadTask { resume(): boolean; snapshot: UploadTaskSnapshot; then( - onFulfilled?: ((a: UploadTaskSnapshot) => any) | null, - onRejected?: ((a: FirebaseStorageError) => any) | null + onFulfilled?: ((snapshot: UploadTaskSnapshot) => any) | null, + onRejected?: ((error: FirebaseStorageError) => any) | null ): Promise; } From 0ac6f06383cce37681edb85af0400312aa9826ea Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 16 Oct 2020 10:28:42 -0700 Subject: [PATCH 3/5] Remove serverResponse field in favor of customData --- packages/firebase/index.d.ts | 7 +-- packages/storage-types/index.d.ts | 9 +--- packages/storage/src/implementation/error.ts | 51 +++++++------------ .../storage/src/implementation/request.ts | 2 +- .../storage/src/implementation/requests.ts | 4 +- 5 files changed, 23 insertions(+), 50 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 0dee698efce..fba8b134f3f 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -7620,12 +7620,7 @@ declare namespace firebase.storage { /** * An error returned by the Firebase Storage SDK. */ - interface FirebaseStorageError { - name: string; - code: string; - message: string; - serverResponse: null | string; - } + interface FirebaseStorageError extends FirebaseError {} interface StorageObserver { next?: NextFn | null; diff --git a/packages/storage-types/index.d.ts b/packages/storage-types/index.d.ts index c8e5201807d..546c182338f 100644 --- a/packages/storage-types/index.d.ts +++ b/packages/storage-types/index.d.ts @@ -16,7 +16,7 @@ */ import { FirebaseApp } from '@firebase/app-types'; -import { CompleteFn, NextFn, Unsubscribe } from '@firebase/util'; +import { CompleteFn, FirebaseError, NextFn, Unsubscribe } from '@firebase/util'; export interface FullMetadata extends UploadMetadata { bucket: string; @@ -85,12 +85,7 @@ export interface UploadMetadata extends SettableMetadata { md5Hash?: string | null; } -export interface FirebaseStorageError { - name: string; - code: string; - message: string; - serverResponse: null | string; -} +export interface FirebaseStorageError extends FirebaseError {} export interface StorageObserver { next?: NextFn | null; diff --git a/packages/storage/src/implementation/error.ts b/packages/storage/src/implementation/error.ts index 98341b71922..66c557e9d3b 100644 --- a/packages/storage/src/implementation/error.ts +++ b/packages/storage/src/implementation/error.ts @@ -1,3 +1,4 @@ +import { FirebaseError } from '@firebase/util'; /** * @license * Copyright 2017 Google LLC @@ -16,53 +17,35 @@ */ import { CONFIG_STORAGE_BUCKET_KEY } from './constants'; -export class FirebaseStorageError implements Error { - private code_: string; - private message_: string; - private serverResponse_: string | null; - private name_: string; +export class FirebaseStorageError extends FirebaseError { + customData: { serverResponse: string | null }; constructor(code: Code, message: string) { - this.code_ = prependCode(code); - this.message_ = 'Firebase Storage: ' + message; - this.serverResponse_ = null; - this.name_ = 'FirebaseError'; - } - - codeProp(): string { - return this.code; + super(prependCode(code), 'Firebase Storage: ' + message); + // Without this, `instanceof FirebaseStorageError`, in tests for example, + // returns false. + Object.setPrototypeOf(this, FirebaseStorageError.prototype); + this.customData = { serverResponse: null }; } codeEquals(code: Code): boolean { - return prependCode(code) === this.codeProp(); - } - - serverResponseProp(): string | null { - return this.serverResponse_; - } - - setServerResponseProp(serverResponse: string | null): void { - this.serverResponse_ = serverResponse; - } - - get name(): string { - return this.name_; - } - - get code(): string { - return this.code_; + return prependCode(code) === this.code; } get message(): string { - if (this.serverResponse_) { - return this.message_ + '\n' + this.serverResponse_; + if (this.customData.serverResponse) { + return this.message + '\n' + this.customData.serverResponse; } else { - return this.message_; + return this.message; } } get serverResponse(): null | string { - return this.serverResponse_; + return this.customData.serverResponse; + } + + set serverResponse(serverResponse: string | null) { + this.customData.serverResponse = serverResponse; } } diff --git a/packages/storage/src/implementation/request.ts b/packages/storage/src/implementation/request.ts index 90868eb6a55..5b553ad3af8 100644 --- a/packages/storage/src/implementation/request.ts +++ b/packages/storage/src/implementation/request.ts @@ -188,7 +188,7 @@ class NetworkRequest implements Request { } else { if (xhr !== null) { const err = unknown(); - err.setServerResponseProp(xhr.getResponseText()); + err.serverResponse = xhr.getResponseText(); if (self.errorCallback_) { reject(self.errorCallback_(xhr, err)); } else { diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index 6eeb75de161..8b1fe288e25 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -114,7 +114,7 @@ export function sharedErrorHandler( } } } - newErr.setServerResponseProp(err.serverResponseProp()); + newErr.serverResponse = err.serverResponse; return newErr; } return errorHandler; @@ -133,7 +133,7 @@ export function objectErrorHandler( if (xhr.getStatus() === 404) { newErr = objectNotFound(location.path); } - newErr.setServerResponseProp(err.serverResponseProp()); + newErr.serverResponse = err.serverResponse; return newErr; } return errorHandler; From 16c4ebe10acecdab3f789b432fda0a9c135a98a2 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 16 Oct 2020 10:39:43 -0700 Subject: [PATCH 4/5] Specify customData structure for FirebaseStorageError type --- packages/firebase/index.d.ts | 4 +++- packages/storage-types/index.d.ts | 4 +++- packages/storage/src/implementation/error.ts | 3 +-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index fba8b134f3f..100cf99e063 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -7620,7 +7620,9 @@ declare namespace firebase.storage { /** * An error returned by the Firebase Storage SDK. */ - interface FirebaseStorageError extends FirebaseError {} + interface FirebaseStorageError extends FirebaseError { + customData: { serverResponse: string | null }; + } interface StorageObserver { next?: NextFn | null; diff --git a/packages/storage-types/index.d.ts b/packages/storage-types/index.d.ts index 546c182338f..de3cc127146 100644 --- a/packages/storage-types/index.d.ts +++ b/packages/storage-types/index.d.ts @@ -85,7 +85,9 @@ export interface UploadMetadata extends SettableMetadata { md5Hash?: string | null; } -export interface FirebaseStorageError extends FirebaseError {} +interface FirebaseStorageError extends FirebaseError { + customData: { serverResponse: string | null }; +} export interface StorageObserver { next?: NextFn | null; diff --git a/packages/storage/src/implementation/error.ts b/packages/storage/src/implementation/error.ts index 66c557e9d3b..47780c177af 100644 --- a/packages/storage/src/implementation/error.ts +++ b/packages/storage/src/implementation/error.ts @@ -18,14 +18,13 @@ import { FirebaseError } from '@firebase/util'; import { CONFIG_STORAGE_BUCKET_KEY } from './constants'; export class FirebaseStorageError extends FirebaseError { - customData: { serverResponse: string | null }; + customData: { serverResponse: string | null } = { serverResponse: null }; constructor(code: Code, message: string) { super(prependCode(code), 'Firebase Storage: ' + message); // Without this, `instanceof FirebaseStorageError`, in tests for example, // returns false. Object.setPrototypeOf(this, FirebaseStorageError.prototype); - this.customData = { serverResponse: null }; } codeEquals(code: Code): boolean { From fb010a6f4946184f9cdfc89f0aebd0acd9787b80 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Mon, 19 Oct 2020 16:52:47 -0700 Subject: [PATCH 5/5] Expose serverResponse directly on FirebaseStorageError --- packages/firebase/index.d.ts | 2 +- packages/storage-types/index.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 100cf99e063..420e8b2eb7b 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -7621,7 +7621,7 @@ declare namespace firebase.storage { * An error returned by the Firebase Storage SDK. */ interface FirebaseStorageError extends FirebaseError { - customData: { serverResponse: string | null }; + serverResponse: string | null; } interface StorageObserver { diff --git a/packages/storage-types/index.d.ts b/packages/storage-types/index.d.ts index de3cc127146..5bd2b98dc05 100644 --- a/packages/storage-types/index.d.ts +++ b/packages/storage-types/index.d.ts @@ -86,7 +86,7 @@ export interface UploadMetadata extends SettableMetadata { } interface FirebaseStorageError extends FirebaseError { - customData: { serverResponse: string | null }; + serverResponse: string | null; } export interface StorageObserver {