Remove serverResponse field in favor of customData

Author: hsubox76

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<T> {
     next?: NextFn<T> | null;

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<T> {
   next?: NextFn<T> | null;

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;
   }
 }
 

packages/storage/src/implementation/request.ts

@@ -188,7 +188,7 @@ class NetworkRequest<T> implements Request<T> {
       } else {
         if (xhr !== null) {
           const err = unknown();
-          err.setServerResponseProp(xhr.getResponseText());
+          err.serverResponse = xhr.getResponseText();
           if (self.errorCallback_) {
             reject(self.errorCallback_(xhr, err));
           } else {

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;

packages/util/src/errors.ts

@@ -69,26 +69,16 @@ export interface ErrorData {
   [key: string]: unknown;
 }
 
-export interface FirebaseError extends Error, ErrorData {
-  // Unique code for error - format is service/error-code-string.
-  readonly code: string;
-
-  // Developer-friendly error message.
-  readonly message: string;
-
-  // Always 'FirebaseError'.
-  readonly name: typeof ERROR_NAME;
-
-  // Where available - stack backtrace in a string.
-  readonly stack?: string;
-}
-
 // Based on code from:
 // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Custom_Error_Types
 export class FirebaseError extends Error {
   readonly name = ERROR_NAME;
 
-  constructor(readonly code: string, message: string) {
+  constructor(
+    readonly code: string,
+    message: string,
+    public customData?: Record<string, unknown>
+  ) {
     super(message);
 
     // Fix For ES5
@@ -125,21 +115,7 @@ export class ErrorFactory<
     // Service Name: Error message (service/code).
     const fullMessage = `${this.serviceName}: ${message} (${fullCode}).`;
 
-    const error = new FirebaseError(fullCode, fullMessage);
-
-    // Keys with an underscore at the end of their name are not included in
-    // error.data for some reason.
-    // TODO: Replace with Object.entries when lib is updated to es2017.
-    for (const key of Object.keys(customData)) {
-      if (key.slice(-1) !== '_') {
-        if (key in error) {
-          console.warn(
-            `Overwriting FirebaseError base field "${key}" can cause unexpected behavior.`
-          );
-        }
-        error[key] = customData[key];
-      }
-    }
+    const error = new FirebaseError(fullCode, fullMessage, customData);
 
     return error;
   }

packages/util/test/errors.test.ts

@@ -15,7 +15,6 @@
  * limitations under the License.
  */
 import { assert } from 'chai';
-import { stub } from 'sinon';
 import { ErrorFactory, ErrorMap, FirebaseError } from '../src/errors';
 
 type ErrorCode =
@@ -60,14 +59,7 @@ describe('FirebaseError', () => {
       e.message,
       "Fake: Could not find file: 'foo.txt' (fake/file-not-found)."
     );
-    assert.equal(e.file, 'foo.txt');
-  });
-
-  it('anonymously replaces template values with data', () => {
-    const e = ERROR_FACTORY.create('anon-replace', { repl_: 'world' });
-    assert.equal(e.code, 'fake/anon-replace');
-    assert.equal(e.message, 'Fake: Hello, world! (fake/anon-replace).');
-    assert.isUndefined(e.repl_);
+    assert.equal(e.customData!.file, 'foo.txt');
   });
 
   it('uses "Error" as template when template is missing', () => {
@@ -88,21 +80,6 @@ describe('FirebaseError', () => {
     );
   });
 
-  it('warns if overwriting a base error field with custom data', () => {
-    const warnStub = stub(console, 'warn');
-    const e = ERROR_FACTORY.create('overwrite-field', {
-      code: 'overwritten code'
-    });
-    assert.equal(e.code, 'overwritten code');
-    // TODO: use sinon-chai for this.
-    assert.ok(
-      warnStub.calledOnceWith(
-        'Overwriting FirebaseError base field "code" can cause unexpected behavior.'
-      )
-    );
-    warnStub.restore();
-  });
-
   it('has stack', () => {
     const e = ERROR_FACTORY.create('generic-error');