Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extensions.service for all subgraph errors #6191

Merged
merged 10 commits into from
Nov 20, 2024
29 changes: 29 additions & 0 deletions .changesets/feat_error_extensions_service.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
### Add `extensions.service` for all subgraph errors ([PR #6191](https://github.com/apollographql/router/pull/6191))

If `include_subgraph_errors` is `true` for a particular subgraph, all errors originating in this subgraph will have the subgraph's name exposed as a `service` extension.

For example, if subgraph errors are enabled, like so:
```yaml title="router.yaml"
include_subgraph_errors:
all: true # Propagate errors from all subgraphs
```
Note: This option is enabled by default in the [dev mode](./configuration/overview#dev-mode-defaults).

And this `products` subgraph returns an error, it will have a `service` extension:

```json
{
"data": null,
"errors": [
{
"message": "Invalid product ID",
"path": [],
"extensions": {
"service": "products"
}
}
]
}
```

By [@IvanGoncharov](https://github.com/IvanGoncharov) in https://github.com/apollographql/router/pull/6191
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ expression: parts
"@"
],
"extensions": {
"code": "FETCH_ERROR"
"code": "FETCH_ERROR",
"service": "reviews"
}
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ expression: response
"currentUser",
"allOrganizations",
2
]
],
"extensions": {
"service": "orga"
}
}
]
}
53 changes: 32 additions & 21 deletions apollo-router/src/plugins/include_subgraph_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,36 +42,47 @@ impl Plugin for IncludeSubgraphErrors {
}

fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService {
Copy link
Member Author

@IvanGoncharov IvanGoncharov Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include_subgraph_errors.rs is the best place I could find to add this functionality:

  1. It's guaranteed that if include_subgraph_errors: false nothing will leak. So, we can't add this code later in the pipeline.
  2. Also, we need to add it as late as possible to maximize the number of errors related to the subgraph handled by this code.

// Search for subgraph in our configured subgraph map.
// If we can't find it, use the "all" value
if !*self.config.subgraphs.get(name).unwrap_or(&self.config.all) {
let sub_name_response = name.to_string();
let sub_name_error = name.to_string();
return service
.map_response(move |mut response: SubgraphResponse| {
if !response.response.body().errors.is_empty() {
// Search for subgraph in our configured subgraph map. If we can't find it, use the "all" value
let include_subgraph_errors = *self.config.subgraphs.get(name).unwrap_or(&self.config.all);

let sub_name_response = name.to_string();
let sub_name_error = name.to_string();
return service
.map_response(move |mut response: SubgraphResponse| {
let errors = &mut response.response.body_mut().errors;
if !errors.is_empty() {
if include_subgraph_errors {
for error in errors.iter_mut() {
error
.extensions
.entry("service")
.or_insert(sub_name_response.clone().into());
}
} else {
tracing::info!("redacted subgraph({sub_name_response}) errors");
for error in response.response.body_mut().errors.iter_mut() {
for error in errors.iter_mut() {
error.message = REDACTED_ERROR_MESSAGE.to_string();
error.extensions = Object::default();
}
}
response
})
// _error to stop clippy complaining about unused assignments...
.map_err(move |mut _error: BoxError| {
}

response
})
.map_err(move |error: BoxError| {
if include_subgraph_errors {
error
} else {
// Create a redacted error to replace whatever error we have
tracing::info!("redacted subgraph({sub_name_error}) error");
_error = Box::new(crate::error::FetchError::SubrequestHttpError {
Box::new(crate::error::FetchError::SubrequestHttpError {
status_code: None,
service: "redacted".to_string(),
reason: "redacted".to_string(),
});
_error
})
.boxed();
}
service
})
}
})
.boxed();
}
}

Expand Down Expand Up @@ -104,7 +115,7 @@ mod test {
use crate::Configuration;

static UNREDACTED_PRODUCT_RESPONSE: Lazy<Bytes> = Lazy::new(|| {
Bytes::from_static(r#"{"data":{"topProducts":null},"errors":[{"message":"couldn't find mock for query {\"query\":\"query($first: Int) { topProducts(first: $first) { __typename upc } }\",\"variables\":{\"first\":2}}","path":[],"extensions":{"test":"value","code":"FETCH_ERROR"}}]}"#.as_bytes())
Bytes::from_static(r#"{"data":{"topProducts":null},"errors":[{"message":"couldn't find mock for query {\"query\":\"query($first: Int) { topProducts(first: $first) { __typename upc } }\",\"variables\":{\"first\":2}}","path":[],"extensions":{"test":"value","code":"FETCH_ERROR","service":"products"}}]}"#.as_bytes())
});

static REDACTED_PRODUCT_RESPONSE: Lazy<Bytes> = Lazy::new(|| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ expression: stream.next_response().await.unwrap()
"path": [
"computer",
"errorField"
]
],
"extensions": {
"service": "computers"
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ expression: stream.next_response().await.unwrap()
"message": "error user 0",
"path": [
"currentUser"
]
],
"extensions": {
"service": "user"
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ expression: stream.next_response().await.unwrap()
"activeOrganization",
"suborga",
0
]
],
"extensions": {
"service": "orga"
}
}
]
},
Expand Down Expand Up @@ -56,7 +59,10 @@ expression: stream.next_response().await.unwrap()
"activeOrganization",
"suborga",
2
]
],
"extensions": {
"service": "orga"
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ expression: stream.next_response().await.unwrap()
"bar"
],
"extensions": {
"code": "NOT_FOUND"
"code": "NOT_FOUND",
"service": "S2"
}
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ expression: stream.next_response().await.unwrap()
"path": [
"currentUser",
"activeOrganization"
]
],
"extensions": {
"service": "orga"
}
}
]
}
14 changes: 14 additions & 0 deletions apollo-router/tests/integration/batching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ async fn it_batches_with_errors_in_single_graph() -> Result<(), BoxError> {
- errors:
- message: expected error in A
path: []
extensions:
service: a
- data:
entryA:
index: 2
Expand Down Expand Up @@ -200,9 +202,13 @@ async fn it_batches_with_errors_in_multi_graph() -> Result<(), BoxError> {
- errors:
- message: expected error in A
path: []
extensions:
service: a
- errors:
- message: expected error in B
path: []
extensions:
service: b
- data:
entryA:
index: 2
Expand Down Expand Up @@ -256,6 +262,7 @@ async fn it_handles_short_timeouts() -> Result<(), BoxError> {
path: []
extensions:
code: REQUEST_TIMEOUT
service: b
- data:
entryA:
index: 1
Expand All @@ -264,6 +271,7 @@ async fn it_handles_short_timeouts() -> Result<(), BoxError> {
path: []
extensions:
code: REQUEST_TIMEOUT
service: b
"###);
}

Expand Down Expand Up @@ -331,16 +339,19 @@ async fn it_handles_indefinite_timeouts() -> Result<(), BoxError> {
path: []
extensions:
code: REQUEST_TIMEOUT
service: b
- errors:
- message: Request timed out
path: []
extensions:
code: REQUEST_TIMEOUT
service: b
- errors:
- message: Request timed out
path: []
extensions:
code: REQUEST_TIMEOUT
service: b
"###);
}

Expand Down Expand Up @@ -568,6 +579,7 @@ async fn it_handles_cancelled_by_coprocessor() -> Result<(), BoxError> {
path: []
extensions:
code: ERR_NOT_ALLOWED
service: a
- data:
entryB:
index: 0
Expand All @@ -576,6 +588,7 @@ async fn it_handles_cancelled_by_coprocessor() -> Result<(), BoxError> {
path: []
extensions:
code: ERR_NOT_ALLOWED
service: a
- data:
entryB:
index: 1
Expand Down Expand Up @@ -725,6 +738,7 @@ async fn it_handles_single_request_cancelled_by_coprocessor() -> Result<(), BoxE
path: []
extensions:
code: ERR_NOT_ALLOWED
service: a
- data:
entryB:
index: 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
source: apollo-router/tests/integration/traffic_shaping.rs
expression: response
---
"{\"data\":null,\"errors\":[{\"message\":\"Your request has been rate limited\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_RATE_LIMITED\"}}]}"
"{\"data\":null,\"errors\":[{\"message\":\"Your request has been rate limited\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_RATE_LIMITED\",\"service\":\"products\"}}]}"
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
source: apollo-router/tests/integration/traffic_shaping.rs
expression: response
---
"{\"data\":null,\"errors\":[{\"message\":\"Request timed out\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_TIMEOUT\"}}]}"
"{\"data\":null,\"errors\":[{\"message\":\"Request timed out\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_TIMEOUT\",\"service\":\"products\"}}]}"
Loading