Skip to content

Document.set() returns a Promise but can also throw an Error that is uncaught by set().catch() #5818

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

Closed
kmorgan8588 opened this issue Dec 16, 2021 · 4 comments
Assignees

Comments

@kmorgan8588
Copy link

[REQUIRED] Describe your environment

  • Operating System version: macOS BigSur v11.6
  • Browser version: Chrome v96.0.4664.110
  • Firebase SDK version: v8.2.0
  • Firebase Product: firestore storage

[REQUIRED] Describe the problem

I found a peculiar issue while using the document.set() method to update some data using firestore.

While the typing for .set() is correct in that it returns a Promise which you can then chain off of with .then and .catches,
I found while forcing a scenario where I wanted to test my error handling, that the .catch doesn't pickup up error handling in a situation that I expected it to.

Steps to reproduce:

To recreate the situation: simply call the .set() method with a map object that includes a field that has an undefined value.
ex:

{
startTime: Firestore.FieldValue.serverTimestamp(),
fail: undefined
}

This is a very contrived example but I knew it should have hit my error handling code, which was my goal.

However, it wasn't until I wrapped the functionality into a try/catch block that I was able to do anything with the error coming out of .set()

So I have a work around and I'm basically just wanting to report this and make sure that this is intended. And if so, ask that the documentation is updated to reflect the fact that multiple styles of catching are necessary to capture any error paths from set()'s usage.

Relevant Code:

return document.set( { 
startTime: Firestore.FieldValue.serverTimestamp(),
 fail: undefined 
})
.then(cb)
.catch(error => {
//handle error code that doesn't occur
})

Workaround:

return new Promise((resolve, reject) => {
try { document.set( { 
startTime: Firestore.FieldValue.serverTimestamp(),
 fail: undefined 
})
.then(resolve)
.catch(reject)
} catch (error) {
 //error handling that does occur from invalid set usage
}
})
@ehsannas
Copy link
Contributor

Thanks for reporting this @kmorgan8588 . I'll take a look.

@ehsannas ehsannas self-assigned this Dec 16, 2021
@schmidt-sebastian
Copy link
Contributor

This is actually intended behavior. We validate user input synchronously and deliberately throw exceptions instead of returning rejected Promises. Exceptions point to behavior in your code that you should avoid. In your production app, your code should ideally not cause exceptions.

We use Promises for errors that cannot be prevented - e.g. when the backend rejects a write.

@kmorgan8588
Copy link
Author

Any possibility of updating the documentation or function interface (intellisense) ? It took a fair amount of sleuthing to understand what was happening here and hopefully, this can save someone some headaches in the future.

Thanks for the prompt reply by the way. I appreciate it.

@schmidt-sebastian
Copy link
Contributor

I will update the documentation.

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

No branches or pull requests

4 participants