From 6121411bf369a814ba156957b4af0c45c303dec0 Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 13 Nov 2020 16:59:44 -0700 Subject: [PATCH] feat: better errors BREAKING CHANGE: No longer throwing a custom ClientError error type Errors attempt to include a better message as the error.message The thrown errors still have the strange amalgamation of request and response properties. --- cjs/index.js | 46 ++++++++++++++++++++++++++++++++-------------- cjs/index.test.js | 4 ++-- esm/index.js | 45 +++++++++++++++++++++++++++++++-------------- esm/index.test.js | 4 ++-- esm/types.js | 28 ---------------------------- package.json | 1 + 6 files changed, 68 insertions(+), 60 deletions(-) delete mode 100644 esm/types.js diff --git a/cjs/index.js b/cjs/index.js index ea12f87..5339a6b 100644 --- a/cjs/index.js +++ b/cjs/index.js @@ -1,6 +1,6 @@ 'use strict'; const fetch = (m => m.__esModule ? /* istanbul ignore next */ m.default : /* istanbul ignore next */ m)(require('node-fetch')) -const { ClientError } = require('./types.js') +const get = (m => m.__esModule ? /* istanbul ignore next */ m.default : /* istanbul ignore next */ m)(require('lodash.get')) class GraphQLClient { constructor (url, options = {}) { @@ -8,34 +8,33 @@ class GraphQLClient { this.options = options } - async rawStringRequest (body) { + async rawStringRequest (requestBody) { // If you need to generate your gql body elsewhere, you can still utilize errors and options. const { headers, ...others } = this.options const response = await fetch(this.url, { method: 'POST', headers: { 'Content-Type': 'application/json', ...headers }, - body, + body: requestBody, ...others }) - const result = await getResult(response) + const responseBody = await getBody(response) - if (response.ok && !result.errors && result.data) { + if (response.ok && !responseBody.errors && responseBody.data) { const { headers, status } = response - return { ...result, headers, status } + return { ...responseBody, headers, status } } else { - const errorResult = typeof result === 'string' ? { error: result } : result + const errorResponseBody = typeof result === 'string' ? { error: responseBody } : responseBody - let bodyObj = body + let requestBodyObject = requestBody try { - bodyObj = JSON.parse(body) + requestBodyObject = JSON.parse(requestBody) } catch (e) { /* Swallow parsing errors */ } - throw new ClientError( - { ...errorResult, status: response.status, headers: response.headers }, - bodyObj - ) + const error = generateError({ errorResponseBody, response, requestBodyObject }) + + throw error } } @@ -102,7 +101,7 @@ function rawStringRequest (url, body, opts) { } exports.rawStringRequest = rawStringRequest -function getResult (response) { +function getBody (response) { const contentType = response.headers.get('Content-Type') if (contentType && contentType.startsWith('application/json')) { return response.json() @@ -110,3 +109,22 @@ function getResult (response) { return response.text() } } + +function generateError ({ errorResponseBody, response, requestBodyObject }) { + // The goal is to capture a real error, with a halfway decent message. + // If there are additional object paths with good errors, we can add them here. + // This coveres apollo. + const message = get(errorResponseBody, 'errors[0].extensions.exception.data.message[0].messages[0].message') || + get(errorResponseBody, 'errors[0].extensions.exception.data.data[0].messages[0].message') || + get(errorResponseBody, 'errors[0].message') || + get(errorResponseBody, 'errors.message') || + get(response, 'statusText') || + 'There was an error with the request.' + const error = new Error(message) + + error.response = { ...errorResponseBody, status: response.status, headers: response.headers } + error.request = requestBodyObject + + return error +} +exports.generateError = generateError diff --git a/cjs/index.test.js b/cjs/index.test.js index 29ab5dc..e0fd238 100644 --- a/cjs/index.test.js +++ b/cjs/index.test.js @@ -169,7 +169,7 @@ tap.test('basic error', async t => { const res = await request(ctx.url, 'x').catch((x) => x) - t.deepEqual(res.message, 'GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{}},"request":{"query":"x"}}') + t.deepEqual(res.message, 'Syntax Error GraphQL request (1:1) Unexpected Name "x"\n\n1: x\n ^\n') }) tap.test('basic error with raw request', async t => { @@ -187,7 +187,7 @@ tap.test('basic error with raw request', async t => { } }) const res = await rawRequest(ctx.url, 'x').catch((x) => x) - t.deepEqual(res.message, 'GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{}},"request":{"query":"x"}}') + t.deepEqual(res.message, 'Syntax Error GraphQL request (1:1) Unexpected Name "x"\n\n1: x\n ^\n') }) tap.test('shut down test server', t => { diff --git a/esm/index.js b/esm/index.js index 71f02e9..7f85bb0 100644 --- a/esm/index.js +++ b/esm/index.js @@ -1,5 +1,5 @@ import fetch from 'node-fetch' -import { ClientError } from './types.js' +import get from 'lodash.get' export class GraphQLClient { constructor (url, options = {}) { @@ -7,34 +7,33 @@ export class GraphQLClient { this.options = options } - async rawStringRequest (body) { + async rawStringRequest (requestBody) { // If you need to generate your gql body elsewhere, you can still utilize errors and options. const { headers, ...others } = this.options const response = await fetch(this.url, { method: 'POST', headers: { 'Content-Type': 'application/json', ...headers }, - body, + body: requestBody, ...others }) - const result = await getResult(response) + const responseBody = await getBody(response) - if (response.ok && !result.errors && result.data) { + if (response.ok && !responseBody.errors && responseBody.data) { const { headers, status } = response - return { ...result, headers, status } + return { ...responseBody, headers, status } } else { - const errorResult = typeof result === 'string' ? { error: result } : result + const errorResponseBody = typeof result === 'string' ? { error: responseBody } : responseBody - let bodyObj = body + let requestBodyObject = requestBody try { - bodyObj = JSON.parse(body) + requestBodyObject = JSON.parse(requestBody) } catch (e) { /* Swallow parsing errors */ } - throw new ClientError( - { ...errorResult, status: response.status, headers: response.headers }, - bodyObj - ) + const error = generateError({ errorResponseBody, response, requestBodyObject }) + + throw error } } @@ -96,7 +95,7 @@ export function rawStringRequest (url, body, opts) { return client.rawStringRequest(body) } -function getResult (response) { +function getBody (response) { const contentType = response.headers.get('Content-Type') if (contentType && contentType.startsWith('application/json')) { return response.json() @@ -104,3 +103,21 @@ function getResult (response) { return response.text() } } + +export function generateError ({ errorResponseBody, response, requestBodyObject }) { + // The goal is to capture a real error, with a halfway decent message. + // If there are additional object paths with good errors, we can add them here. + // This coveres apollo. + const message = get(errorResponseBody, 'errors[0].extensions.exception.data.message[0].messages[0].message') || + get(errorResponseBody, 'errors[0].extensions.exception.data.data[0].messages[0].message') || + get(errorResponseBody, 'errors[0].message') || + get(errorResponseBody, 'errors.message') || + get(response, 'statusText') || + 'There was an error with the request.' + const error = new Error(message) + + error.response = { ...errorResponseBody, status: response.status, headers: response.headers } + error.request = requestBodyObject + + return error +} diff --git a/esm/index.test.js b/esm/index.test.js index f001a6c..c11f875 100644 --- a/esm/index.test.js +++ b/esm/index.test.js @@ -168,7 +168,7 @@ tap.test('basic error', async t => { const res = await request(ctx.url, 'x').catch((x) => x) - t.deepEqual(res.message, 'GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{}},"request":{"query":"x"}}') + t.deepEqual(res.message, 'Syntax Error GraphQL request (1:1) Unexpected Name "x"\n\n1: x\n ^\n') }) tap.test('basic error with raw request', async t => { @@ -186,7 +186,7 @@ tap.test('basic error with raw request', async t => { } }) const res = await rawRequest(ctx.url, 'x').catch((x) => x) - t.deepEqual(res.message, 'GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{}},"request":{"query":"x"}}') + t.deepEqual(res.message, 'Syntax Error GraphQL request (1:1) Unexpected Name "x"\n\n1: x\n ^\n') }) tap.test('shut down test server', t => { diff --git a/esm/types.js b/esm/types.js deleted file mode 100644 index d753fe4..0000000 --- a/esm/types.js +++ /dev/null @@ -1,28 +0,0 @@ -export class ClientError extends Error { - constructor (response, request) { - const message = `${extractMessage(response)}: ${JSON.stringify({ - response, - request - })}` - - super(message) - - Object.setPrototypeOf(this, ClientError.prototype) - - this.response = response - this.request = request - - // this is needed as Safari doesn't support .captureStackTrace - if (typeof Error.captureStackTrace === 'function') { - Error.captureStackTrace(this, ClientError) - } - } -} - -function extractMessage (response) { - try { - return response.errors[0].message - } catch (e) { - return `GraphQL Error (Code: ${response.status})` - } -} diff --git a/package.json b/package.json index fe847fc..9da1c61 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ }, "homepage": "https://github.com/little-core-labs/gqlr#readme", "dependencies": { + "lodash.get": "^4.4.2", "node-fetch": "^2.6.0" }, "devDependencies": {