Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix storage-types #3945

Merged
merged 5 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7617,6 +7617,19 @@ declare namespace firebase.storage {
md5Hash?: string | null;
}

/**
* An error returned by the Firebase Storage SDK.
*/
interface FirebaseStorageError extends FirebaseError {
customData: { serverResponse: string | null };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you have a getter serverResponse, customData feels like the internal implementation and you can just expose the getter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

interface StorageObserver<T> {
next?: NextFn<T> | null;
error?: (error: FirebaseStorageError) => void | null;
complete?: CompleteFn | null;
}

/**
* Represents the process of uploading an object. Allows you to monitor and
* manage the upload.
Expand All @@ -7630,7 +7643,7 @@ declare namespace firebase.storage {
/**
* Equivalent to calling `then(null, onRejected)`.
*/
catch(onRejected: (a: Error) => any): Promise<any>;
catch(onRejected: (error: FirebaseStorageError) => any): Promise<any>;
/**
* Listens for events on this task.
*
Expand Down Expand Up @@ -7730,7 +7743,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.
Expand All @@ -7743,10 +7756,10 @@ declare namespace firebase.storage {
on(
event: firebase.storage.TaskEvent,
nextOrObserver?:
| Partial<firebase.Observer<UploadTaskSnapshot>>
| StorageObserver<UploadTaskSnapshot>
| null
| ((a: UploadTaskSnapshot) => any),
error?: ((a: Error) => any) | null,
| ((snapshot: UploadTaskSnapshot) => any),
error?: ((error: FirebaseStorageError) => any) | null,
complete?: firebase.Unsubscribe | null
): Function;
/**
Expand All @@ -7771,8 +7784,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<any>;
}

Expand Down
28 changes: 19 additions & 9 deletions packages/storage-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
* limitations under the License.
*/

import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types';
import { Observer, Unsubscribe } from '@firebase/util';
import { FirebaseApp } from '@firebase/app-types';
import { CompleteFn, FirebaseError, NextFn, Unsubscribe } from '@firebase/util';

export interface FullMetadata extends UploadMetadata {
bucket: string;
Expand Down Expand Up @@ -48,7 +48,7 @@ export interface Reference {
metadata?: UploadMetadata
): UploadTask;
root: Reference;
storage: Storage;
storage: FirebaseStorage;
toString(): string;
updateMetadata(metadata: SettableMetadata): Promise<FullMetadata>;
listAll(): Promise<ListResult>;
Expand Down Expand Up @@ -85,24 +85,34 @@ export interface UploadMetadata extends SettableMetadata {
md5Hash?: string | null;
}

interface FirebaseStorageError extends FirebaseError {
customData: { serverResponse: string | null };
}

export interface StorageObserver<T> {
next?: NextFn<T> | null;
error?: (error: FirebaseStorageError) => void | null;
complete?: CompleteFn | null;
}

export interface UploadTask {
cancel(): boolean;
catch(onRejected: (a: Error) => any): Promise<any>;
catch(onRejected: (error: FirebaseStorageError) => any): Promise<any>;
on(
event: TaskEvent,
nextOrObserver?:
| Partial<Observer<UploadTaskSnapshot>>
| StorageObserver<UploadTaskSnapshot>
| null
| ((a: UploadTaskSnapshot) => any),
error?: ((a: Error) => any) | null,
| ((snapshot: UploadTaskSnapshot) => any),
error?: ((a: FirebaseStorageError) => any) | null,
complete?: Unsubscribe | null
): Function;
pause(): boolean;
resume(): boolean;
snapshot: UploadTaskSnapshot;
then(
onFulfilled?: ((a: UploadTaskSnapshot) => any) | null,
onRejected?: ((a: Error) => any) | null
onFulfilled?: ((snapshot: UploadTaskSnapshot) => any) | null,
onRejected?: ((error: FirebaseStorageError) => any) | null
): Promise<any>;
}

Expand Down
50 changes: 16 additions & 34 deletions packages/storage/src/implementation/error.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FirebaseError } from '@firebase/util';
/**
* @license
* Copyright 2017 Google LLC
Expand All @@ -16,53 +17,34 @@
*/
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 } = { serverResponse: 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);
}

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

Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/implementation/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions packages/storage/src/implementation/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export function sharedErrorHandler(
}
}
}
newErr.setServerResponseProp(err.serverResponseProp());
newErr.serverResponse = err.serverResponse;
return newErr;
}
return errorHandler;
Expand All @@ -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;
Expand Down