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

How should we handle errors on manual requests? #83

Closed
DarynHolmes opened this issue Nov 14, 2019 · 6 comments
Closed

How should we handle errors on manual requests? #83

DarynHolmes opened this issue Nov 14, 2019 · 6 comments

Comments

@DarynHolmes
Copy link

DarynHolmes commented Nov 14, 2019

Thanks for the library!

I have a question around this example:

import useAxios from 'axios-hooks'

function App() {
  const [{ data: getData, loading: getLoading, error: getError }] = useAxios(
    'https://api.myjson.com/bins/820fc'
  )

  const [
    { data: putData, loading: putLoading, error: putError },
    executePut
  ] = useAxios(
    {
      url: 'https://api.myjson.com/bins/820fc',
      method: 'PUT'
    },
    { manual: true }
  )

  function updateData() {
    executePut({
      data: {
        ...getData,
        updatedAt: new Date().toISOString()
      }
    })
  }

  if (getLoading || putLoading) return <p>Loading...</p>
  if (getError || putError) return <p>Error!</p>

  return (
    <div>
      <button onClick={updateData}>update data</button>
      <pre>{JSON.stringify(putData || getData, null, 2)}</pre>
    </div>
  )
}

If there is an error calling executePut then we could end up with the following error

createError.js:17 Uncaught (in promise) Error: Request failed with status code 500
    at createError (createError.js:17)

This does not show if I use

      data: {
        ...getData,
        updatedAt: new Date().toISOString()
      }
    }).catch(() => {})

I was not expecting an error to be thrown, as the hook sets the error. Is this how I should handle this error, or am I doing something wrong?

@simoneb
Copy link
Owner

simoneb commented Nov 14, 2019

Fair point. It's been requested in #51 that the manual fetch function returns the promise, which makes sense to me because if you do a manual fetch you may also want more control about what happens when the promise resolves/rejects. But this also means that if there's a rejection and you're not interested into it you need to swallow it somehow.

Unfortunately I don't see a solution that can cover both cases. Ideas?

@DarynHolmes
Copy link
Author

DarynHolmes commented Nov 14, 2019

I thought it might be a deliberate design choice. It can be quite nice to get the actual promise.

I guess you could pass in another option like this:

          url: '/login',
          method: 'POST'
        },
        { 
            manual: true,
            void: true // or returnPromise: false
         }
    )

There is no need to over complicate this for me, at the moment I am using:

const silenceError = (promise) => promise.catch(() => {})
...
...
...
    const login = () => {
      silenceError(postLogin({
        data: {
          username,
          password
        }
      }))
    }

@simoneb
Copy link
Owner

simoneb commented Nov 14, 2019

I guess the additional configuration option is one possible way but it doesn't feel like it deserves so much attention at the current time, I would like to keep things simple. If there's demand for this feature I may reconsider it. Thanks for reporting though!

@simoneb simoneb closed this as completed Nov 14, 2019
@DarynHolmes
Copy link
Author

I might just do this

const silenceError = (fn) => () => fn().catch(() => {})
    const [{ data, loading, error }, postLogin] = useAxios({
          url: '/login',
          method: 'POST'
        },
        { manual: true }
    )

    const login = silenceError(() => {
      return postLogin({
        data: {
          username,
          password
        }
      })
    })

@DarynHolmes
Copy link
Author

DarynHolmes commented Nov 14, 2019

Or just catch the promise in a standard way....

Thanks again for the library.

@sujeet-agrahari

This comment was marked as duplicate.

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

No branches or pull requests

3 participants