Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Suggestion: Return error objects instead of error message string #1479

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tomcatmwi
Copy link

Here's a quick and somewhat quick and dirty implementation for Storage only. If you only return the error message string from Firebase, it's difficult to process in the app. I suggest to return the exception or error object instead:

onFailure: exception => { reject(exception); }

That will carry the error code and it can be extracted with .errorCode() if needed, and existing code which expects a string also won't break. Seems to work on Android but I didn't test it on iOS.

@@ -85,7 +85,7 @@ export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {

const onFailureListener = new gmsTasks.OnFailureListener({
onFailure: exception => {
reject("Upload failed. " + exception);
reject(exception);
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure it's wise to return the native Java Exception object here. It used to be a String representation. Not saying that's perfect, but this is not better imo.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure either, that's why it's a rough fix. But if not the entire object, the Google error code may still work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants