Apollo-server: [apollo-datasource-rest] REST server's error JSON response is obscured by thrown ApolloError

[BenJenkinson]

I'm using apollo-server v2's new apollo-datasource-rest package to wrap an existing REST API and expose it over GraphQL.

When my API request results in an error, 400/403/404 etc, the API returns its own { "error": "..." } JSON response explaining the error. (Missing fields, invalid data, params that can't be used together, etc..)

At the moment, any non-2XX response when calling this.get('/my/api') throws an ApolloError which then doesn't contain the response to inspect.

I would like to be able to access it, so that I can throw a UserInputError as the resolvers do, but with the message(s) from the API.

Is there a way I can accomplish this without copying/sub-classing everything that apollo-datasource-rest is meant to provide for me?

[martijnwalraven]

I just realize I haven't documented this yet, but there is a didReceiveErrorResponse method on RESTDataSource that you can override to return your own error response. It is invoked on any non-2xx status code and gets passed the response so you can do with it whatever you need and return something that makes sense for you.

I'm surprised the default implementation doesn't include the response from the server in your case however, because it sets it as the message of the ApolloError. Maybe we need a smarter default that knows how to handle some API error conventions?

[BenJenkinson]

Thanks for replying :)

Yes, sorry, the default implementation does include it, but it encodes the JSON into a string with the status code and status text. (Example below)

{
  "data": {
    "moon-landing-sites": null
  },
  "errors": [
    {
      "message": "404 Not Found: { \"errors\": [{ \"message\": \"Houston, we have a problem.\" }] }",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "moon-landing-sites"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "stacktrace": [
            "Error: 404 Not Found: { \"errors\": [{ \"message\": \"Houston, we have a problem.\" }] }",
            "    at MoonLandingSitesAPI.<anonymous> (\\server\\node_modules\\apollo-datasource-rest\\dist\\RESTDataSource.js:34:23)",
            "    at Generator.next (<anonymous>)",
            "    at fulfilled (\\server\\node_modules\\apollo-datasource-rest\\dist\\RESTDataSource.js:4:58)",
            "    at <anonymous>",
            "    at process._tickCallback (internal/process/next_tick.js:188:7)"
          ]
        }
      }
    }
  ]
}

It's just a little inconvenient to have to parse the JSON back out of the message property in order to do something with it.

For the default implementation; I think it might be nice if when the status code is non-200, and the response type is application/json, we take the response JSON and add it to the "additional properties" that an ApolloError can contain.

Then the response could look like this:

{
  "data": {
    "moon-landing-sites": null
  },
  "errors": [
    {
      "message": "Error: 404 Not Found",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "moon-landing-sites"
      ],
      "extensions": {
        "code": "NOT_FOUND",
        "httpStatusCode": 404,
        "response": {
          "errors": [
            {
              "message": "Houston, we have a problem."
            }
          ]
        },
        "exception": {
          "stacktrace": [
            "Error: 404 Not Found",
            "    at MoonLandingSitesAPI.<anonymous> (\\server\\node_modules\\apollo-datasource-rest\\dist\\RESTDataSource.js:34:23)",
            "    at Generator.next (<anonymous>)",
            "    at fulfilled (\\server\\node_modules\\apollo-datasource-rest\\dist\\RESTDataSource.js:4:58)",
            "    at <anonymous>",
            "    at process._tickCallback (internal/process/next_tick.js:188:7)"
          ]
        }
      }
    }
  ]
}

What do you think?

[martijnwalraven]

I like that. But I would probably put httpStatusCode under response as statusCode.

I'm a little confused by "path": [ "httpStatus" ]. Is that part of the query? How does it relate to the "moon-landing-sites" under data?

(I also think a standard ApolloError subclass NotFoundError with a NOT_FOUND code would be a nice addition.)

[BenJenkinson]

Sorry, ignore "path": [ "httpStatus" ], I should've changed that to "path": [ "moon-landing-sites" ] when I copy-pasted my examples.

Ok, so I moved httpStatusCode under response as statusCode. I also added the statusText property that would normally accompany the status code; and nested the actual response body as data so we don't have any problems if the response body is an array, or has its own statusCode/statusText property.

{
  "data": {
    "moon-landing-sites": null
  },
  "errors": [
    {
      "message": "Error: 404 Not Found",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "moon-landing-sites"
      ],
      "extensions": {
        "code": "NOT_FOUND",
        "response": {
          "statusCode": 404,
          "statusText": "Not Found",
          "data": {
            "errors": [
              {
                "message": "Houston, we have a problem."
              }
            ]
          }
        },
        "exception": {
          "stacktrace": [
            "Error: 404 Not Found",
            "    at MoonLandingSitesAPI.<anonymous> (\\server\\node_modules\\apollo-datasource-rest\\dist\\RESTDataSource.js:34:23)",
            "    at Generator.next (<anonymous>)",
            "    at fulfilled (\\server\\node_modules\\apollo-datasource-rest\\dist\\RESTDataSource.js:4:58)",
            "    at <anonymous>",
            "    at process._tickCallback (internal/process/next_tick.js:188:7)"
          ]
        }
      }
    }
  ]
}

[aonamrata]

FYI, with latest version didReceiveErrorResponse method does not exist, user errorFromResponse
https://github.com/apollographql/apollo-server/commit/c200df05d339fa551033bd852b652802d7aded0d#diff-71fa4a920896abff1b3ba641537b66b8

[jalovatt]

For some reason I'm unable to this.get(...).catch or try { this.get(...) } catch (err) ... with RESTDataSource - errors just go straight up to Apollo.

For anyone else who might stumble across this and is in a similar situation, we added a simple override to avoid throwing for the errors we want to handle internally, and then just have the method that made the request check for .error in the response:

  async didReceiveResponse(response, _request) {
    if (response.status === 429) { return { error: { response, request: _request } }; }

    return super.didReceiveResponse(response, _request);
  }