query: Thoughts on mutate function not handling rejected promises

Hey Tanner! Loving this library so far. Just spend the last little but with a snag on the fact that mutate propagates rejected promises. No big deal because I can handle those errors myself, however I don’t see any reason for react-query to not just handle them for us. For example

import React from 'react'
import ReactDOM from 'react-dom'
import {useMutation} from 'react-query'

function failingMutation() {
  return new Promise((resolve, reject) => {
    setTimeout(() => {
      reject(new Error('oh no'))
    }, 1000)
  })
}

function App() {
  const [mutate, {error}] = useMutation(failingMutation)

  async function handleClick() {
    try {
      await mutate()
    } catch (e) {
      // ignore this because useMutation gives me the
      // error that I need to display anyway.
    }
  }

  return (
    <div>
      <div>{error ? error.message : 'no error yet'}</div>
      <button onClick={handleClick}>Click me</button>
    </div>
  )
}

ReactDOM.render(<App />, document.getElementById('root'))

The point I’m trying to make here is that there’s no value in useMutation not handling the rejected promise for us because it gives us the error anyway.

As I was developing this issue, I thought that maybe this could be useful for having ErrorBoundaries handle those errors, but then I realized that because the error is happening outside of React’s callstack, a failed promise wouldn’t get handled by error boundaries anyway. So react-query could make that happen and that might be useful, but I would argue that it would be better to not, and allow users to just do this instead:

React.useEffect(() => {
  if (error) throw error
}, [error])

Or maybe this could be something as an opt-in option for useMutation.

Either way, I think mutate needs to not propagate rejected promises. Otherwise we have a bit of unnecessary boilerplate to make sure we don’t have unhandled rejections.

What do you think?

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 19
  • Comments: 22 (5 by maintainers)

Most upvoted comments

I came to this issue because I was following the docs, and my try/catch wasn’t catching even though my axios query was definitely failing with a 400 error.

Docs should be updated from useMutation(pingMutation) to useMutation(pingMutation, { throwOnError: true }) or the example should be changed to not use try/catch. Happy to submit pull request if you let me know which way you’d like to go.

Released in v0.4.0! Check out the changelog, docs, or this commit for more information: https://github.com/tannerlinsley/react-query/commit/c4bcdac16b0daf19994e055ddd562e89523d3d6e

@coodoo please provide a codesandbox reproduction. catchRejections is not something I am familiar with, which means it doesn’t exist for at least 2 years 😅.

In v3, the mutation functions were split into mutate and mutateAsync:

  • mutate returns nothing and catches errors
  • mutateAsync returns a Promise, so you need to catch errors manually.

You can read about the differences here: https://tkdodo.eu/blog/mastering-mutations-in-react-query#mutate-or-mutateasync

Hmm I think there are use cases for getting a promise still like in-scope side effects for error handling where you called the mutation. However I would be 100% in on adding an option and global default option that would make these throw or not. That would also make it a backwards compatible change too until we decide to change the default to not throw in the next major version. Thoughts? On Jan 31, 2020, 5:13 PM -0700, Kent C. Dodds notifications@github.com, wrote:

Hey Tanner! Loving this library so far. Just spend the last little but with a snag on the fact that mutate propagates rejected promises. No big deal because I can handle those errors myself, however I don’t see any reason for react-query to not just handle them for us. For example import React from ‘react’ import ReactDOM from ‘react-dom’ import {useMutation} from ‘react-query’

function failingMutation() { return new Promise((resolve, reject) => { setTimeout(() => { reject(new Error(‘oh no’)) }, 1000) }) }

function App() { const [mutate, {error}] = useMutation(failingMutation)

async function handleClick() { try { await mutate() } catch (e) { // ignore this because useMutation gives me the // error that I need to display anyway. } }

return (

<div> <div>{error ? error.message : 'no error yet'}</div> <button onClick={handleClick}>Click me</button> </div> ) }

ReactDOM.render(<App />, document.getElementById(‘root’)) The point I’m trying to make here is that there’s no value in useMutation not handling the rejected promise for us because it gives us the error anyway. As I was developing this issue, I thought that maybe this could be useful for having ErrorBoundaries handle those errors, but then I realized that because the error is happening outside of React’s callstack, a failed promise wouldn’t get handled by error boundaries anyway. So react-query could make that happen and that might be useful, but I would argue that it would be better to not, and allow users to just do this instead: React.useEffect(() => { if (error) throw error }, [error]) Or maybe this could be something as an opt-in option for useMutation. Either way, I think mutate needs to not propagate rejected promises. Otherwise we have a bit of unnecessary boilerplate to make sure we don’t have unhandled rejections. What do you think? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Thanks @TkDodo for the quick reply, mutateAsync is exactly the thing I need to manually catch the error instead of letting it showing up in the devtools console! 🙌

I’m in favor of that 👍

That is because react-query logs to the console per default, which you can override: https://react-query.tanstack.com/reference/setLogger

Oh wow! Thank you so much @TkDodo for answering and totally helping me out. You are a total gem!

Maybe it was just me being easily confused, but I feel like adding some of that information to the error-handling page of docs would be great. It seems unlikely to accidentally stumble upon setLogger api page, at least to me.

Again thanks a bunch 😃

That is because react-query logs to the console per default, which you can override: https://react-query.tanstack.com/reference/setLogger

Yes please if at all possible, amazing library but this def caught me by surprise and brought me here

this looks pretty outdated. Only .mutateAsync needs try/catch, the example should work as-is