apollographql / router

A configurable, high-performance routing runtime for Apollo Federation 🚀
https://www.apollographql.com/docs/router/
Other
795 stars 266 forks source link

Ability to redact all Router-specific error messages #4378

Open lleadbet opened 9 months ago

lleadbet commented 9 months ago

Is your feature request related to a problem? Please describe.

The router by default will respond with a number of different error messages that may not be appropriate for external consumption. For example:

"errors": [
    {
      "message": "HTTP fetch failed from 'ActiveIQ-Graph-Dev-API': 401: Unauthorized",
      "extensions": {
        "code": "SUBREQUEST_HTTP_ERROR",
        "service": "ActiveIQ-Graph-Dev-API",
        "reason": "401: Unauthorized",
       "http": {
          "status": 401
        }
      }
    },
    {
      "message": "Expected a token!",
      "extensions": {
        "code": "UNAUTHENTICATED"
      }
    }
 ]

Which exposes the fact that A) this is a supergraph and B) some information about the subgraph names. While it's not responding with any revealing information, other connection issues (e.g. DNS) will respond with more data that is not ideal to expose:

{
      "message": "HTTP fetch failed from 'shipping': error trying to connect: dns error: no record found for Query { name: Name(\"gql.apollographl.com.localdomain.\"), query_type: AAAA, query_class: IN }",
      "path": [
        "user",
        "orders",
        "@"
      ],
      "extensions": {
        "code": "SUBREQUEST_HTTP_ERROR",
        "service": "shipping",
        "reason": "error trying to connect: dns error: no record found for Query { name: Name(\"gql.apollographl.com.localdomain.\"), query_type: AAAA, query_class: IN }"
      }
    }

Which exposes the address trying to be reached, even if that address doesn't actually exist.

With this in mind, it'd be ideal to hide all router-specific messaging in the errors as to avoid these cases altogether. Additionally, and probably most importantly, these messages are not affected by the:

include_subgraph_errors:
  all: false

Setting, so it's impossible to remove without the help of Rhai/coprocessor.

Describe the solution you'd like

A new configuration block to disable router-specific error messages. Proposed example:

inlcude_errors:
  subgraphs: 
    all: true
    shipping: false
  router: true
  query_planner: false

Where we can now disable errors based on the provider - and potentially at a component (e.g query planner, networking, auth, etc) level.

Describe alternatives you've considered

Rhai does work for this in the meantime:

let subgraph_errors = [];

fn supergraph_service(service){
  service.map_response(|response|{
    response.body.errors = response.body.errors.filter(|error|{
      let e = json::encode(error);
      if (subgraph_errors.contains(e)){
        return false;
      };
      return true;
    });
  })
}

fn subgraph_service(service,subgraph){
  service.map_response(|response|{
    subgraph_errors += response.body.errors.map(|error|{
      json::encode(error)
    });
  })
}

Additional context n/a

garypen commented 8 months ago

By default, the router doesn't include subgraph errors, so it seems odd that you are seeing these. Is router flag --dev set for these invocations? If so, that may be what is causing subgraph errors to be included.

lleadbet commented 8 months ago
image

This is still an issue with:

include_subgraph_errors:
  all: false

set in the config without --dev enabled. Removing networking errors is probably not something the client cares about here, only ops folks.