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

Wrong type information for firebase.storage.UploadTask.on(.., .., error, ..) #1515

Closed
ilsergente1993 opened this issue Feb 2, 2019 · 9 comments · Fixed by #3945
Closed

Comments

@ilsergente1993
Copy link

Describe your environment

  • Operating System version: win10
  • Browser version: Chrome 71.0.3578.98
  • Firebase SDK version: 5.8.0
  • Firebase Product: Storage

Describe the problem

The firebase.storage.UploadTask interface has the method on( .., .., error?: ((a: Error) => any) | null, ..).
Checking the error code Error interface doesn't have the code property.

Steps to reproduce:

Using the example provided here, in the callback function for error a switch-case statement is used to switch among the error codes. The problem is that code is not recognized as property.

...
switch (error.code) {
    case 'storage/unauthorized':
      // User doesn't have permission to access the object
      break;
...

Possible solution

I suppose the solution is to use FirebaseError instead of Error line 720 of type definition.

error?: ((a: Error) => any) | null,

Thanks

@romain-faust
Copy link

romain-faust commented Feb 7, 2019

The problem is also present with Firestore's types:

@ilsergente1993
Copy link
Author

Waiting for the solution, a workaround could be the explicit casting as follow:

var error = <Firebase.FirebaseError>e;

I guess this will work in the same way with your types @romain-faust

Hope this helps
Ciao

@mikelehen
Copy link
Contributor

Discussing internally for Firestore, changing Error to FirestoreError sounds like a good idea as long as this doesn't cause a breaking change for anybody (hopefully not, but I'm 100% sure how TypeScript deals with this).

We also need to audit our code to make sure we really do guarantee that it's always a FirestoreError.

@cmditch
Copy link
Contributor

cmditch commented Jul 13, 2020

Any progress on this?

Fixing this on the Firestore types would be nice as well.

@dconeybe
Copy link
Contributor

Thank you for bringing this back to our attention. We are re-evaluating the fix and will update this issue once a decision is made.

@rtman
Copy link

rtman commented Jul 21, 2020

Doing this for now

      (error) => {
        const firebaseError = error as firebase.FirebaseError;

        // A full list of error codes is available at
        // https://firebase.google.com/docs/storage/web/handle-errors
        switch (firebaseError.code) {
          case 'storage/unauthorized':
            // User doesn't have permission to access the object

            break;

          case 'storage/canceled':
            // User canceled the upload
            break;

          case 'storage/unknown':
            // Unknown error occurred, inspect error.serverResponse
            break;
        }
    }

@hsubox76 hsubox76 self-assigned this Jul 21, 2020
@dconeybe
Copy link
Contributor

Update: The changes to the Firestore API are being reviewed internally. This API review process is standard for any changes to the public API. If approved, I'll merge #3418. I'm aiming for the first week of August 2020 to have it merged.

@dconeybe
Copy link
Contributor

Update: Make that the 2nd week of August 2020 :)

@dconeybe
Copy link
Contributor

dconeybe commented Sep 9, 2020

The fix for Firestore has been merged: #3418.

The fix for firebase.storage is still TODO.

@dconeybe dconeybe removed their assignment Sep 9, 2020
@firebase firebase locked and limited conversation to collaborators Nov 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants