[Discussion] Better error handling

[schickling]

Error handling is currently not ideal and I'd love to hear ideas how to handle errors in a better way.

This is the current way:

import { request } from 'graphql-request'

const wrongQuery = `{
  some random stuff
}`

request('my-endpoint', query)
  .then(data => console.log(data))
  .catch(err => {
    console.log(err.response.errors) // GraphQL response errors
    console.log(err.response.data) // Response data if available
  })

The implementation can be found here.

[IvanGoncharov]

@schickling I think there are several types of error:

• Fetch failed on client side (can't resolve hostname, network is down, etc.)
• Server returned non 2xx code
• Server returned 2xx code but payload is incorrect (Content-Type is not application/json, JSON.parse failed)
• GraphQL request failed completely (data is missing, errors is present)
• GraphQL request partly failed (both data and errors are present)

It worth to write test cases for every type and then make sure that every error has human readable error message and additional fields if needed. If you need help I can look into it closer to the end of this week.

[schickling]

Thanks a lot for that thorough list of possibilities. We'd definitely appreciate a PR for improve error handling and increase test coverage.

One goal for the current way of error handling was to provide as much context (query, server response, url) as possible when logging the error.

[m3nu]

What's the current best practice for error handling? My goal is to find the most meaningful error on why something went wrong. Currently I use this as "wrapper": (lightly edited)

async function request(query, vars) {
    try {
        const data = await graphQLClient.request(print(query), vars);
        return data;
    } catch (e) {
        if('response' in e) {  // Use GQL error if we got a response.
            let error = e.response.errors[0]
            if (error.message === 'Not authenticated.') {  // Special case of expired session
                store.dispatch('auth/sessionExpired');
            } else {
                throw(error)  // Or throw GraphQL error with meaningful error
            }
        }
        else {  // No GQL response, could be network error.
            throw(e)
        }
    }
  }

[johnhaup]

It would be great to be able to opt-out of having an error thrown if the response contains an error so we could handle the data returned outside of a catch block and handle the error as we see appropriate.

[philip-nicholls]

@johnhaup You should use errorPolicy for that

https://www.npmjs.com/package/graphql-request#errorpolicy

[jasonkuhrt]

I've come up with a new system to overhaul this library here https://github.com/jasonkuhrt/graphql-request/issues/509.