GraphQL API does not always return a valid response

[LeBenLeBen]

Bug report

Required System information

• Node.js version: 14.20.1
• NPM version: 6.14.17
• Strapi version: 4.4.4
• Database: Postgres
• Operating system: Debian 10

Describe the bug

It happens that Strapi returns responses that do not comply to what the docs says nor the GraphQL spec. This seems to happen particularly in the context of errors.

This is a problem because it prevents popular error handlers such as Apollo Error Link or Urql errorExchange from catching and acting on them.

Steps to reproduce the behavior

Execute any request against the GraphQL API with an invalid token, for example this query:

query {
  me {
    id
  }
}

With the following headers:

{"Authorization": "Bearer invalid"}

Will result in the following response:

{
  "error": {
    "data": null,
    "error": {
      "status": 401,
      "name": "UnauthorizedError",
      "message": "Missing or invalid credentials",
      "details": {}
    }
  }
}

Which does not comply with GraphQL spec. It looks like the response we would expect from the REST API. However, if you pass no authorization header, you get a properly formatted response:

{
  "errors": [
    {
      "message": "Forbidden access",
      "extensions": {
        "error": {
          "name": "ForbiddenError",
          "message": "Forbidden access",
          "details": {}
        },
        "code": "FORBIDDEN"
      }
    }
  ],
  "data": {
    "me": null
  }
}

Expected behavior

All responses returned by the GraphQL API must comply with the GraphQL spec to be handled easily and properly.

Additional context

I discovered this issue while migrating to Strapi 4. It’s a problem for me because when a user come back to the application later and its token has expired, it doesn’t allow me to catch the error and automatically log out the user.

Relates to #8995 and #9052

[derrickmehaffy]

@alexandrebodin / @Convly I'm not entirely sure what is intended here so I marked it as a bug and as a discussion but I need clarity as to what is intended or not.

[amerikan]

@derrickmehaffy I think what should be happening is that instead of returning this:

{
  "error": {
    "data": null,
    "error": {
      "status": 401,
      "name": "UnauthorizedError",
      "message": "Missing or invalid credentials",
      "details": {}
    }
  }
}

it should return something like this format:

{
  "errors": [
    {
        "message": "Missing or invalid credentials",
        "extensions": {
            "error": {
                "name": "UnauthorizedError",
                "message": "Missing or invalid credentials",
                "details": {}
           },
           "code": "UNAUTHORIZED"
        }
    }
  ],
 "data": {
    "me": null
  }
}

I dug into this a bit, and I believe it's currently working the way it is because of the middleware order. It first (1) checks to see if user is authenticated THEN (2) the Apollo graphql middleware kicks in.

So since it fails authentication due to bearer token missing, then it never gets to (2) and instead shows the error for (1) which is just the HTTP error, not the Grahpql error.

https://github.com/strapi/strapi/blob/82035f18c9ca7fc0c94ffba5f2b36ca68b620edd/packages/plugins/graphql/server/bootstrap.js#L96-L126