set the subgraph error path if not present

.changesets/fix_geal_subgraph_error_path.md

@@ -0,0 +1,5 @@
+### set the subgraph error path if not present ([PR #5773](https://github.com/apollographql/router/pull/5773))
+
+This fixes subgraph response conversion to set the error path in all cases. For some network level errors, the subgraph service was not setting the path
+
+By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/5773

apollo-router/src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap

@@ -21,6 +21,10 @@ expression: parts
     "errors": [
       {
         "message": "couldn't find mock for query {\"query\":\"query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{__typename id product{__typename upc}}}}}\",\"operationName\":\"TopProducts__reviews__1\",\"variables\":{\"representations\":[{\"__typename\":\"Product\",\"upc\":\"1\"},{\"__typename\":\"Product\",\"upc\":\"2\"}]}}",
+        "path": [
+          "topProducts",
+          "@"
+        ],
         "extensions": {
           "code": "FETCH_ERROR"
         }

apollo-router/src/plugins/include_subgraph_errors.rs

@@ -104,19 +104,20 @@ 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 ErrorTopProducts__products__0($first:Int){topProducts(first:$first){__typename upc name}}\",\"operationName\":\"ErrorTopProducts__products__0\",\"variables\":{\"first\":2}}","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 ErrorTopProducts__products__0($first:Int){topProducts(first:$first){__typename upc name}}\",\"operationName\":\"ErrorTopProducts__products__0\",\"variables\":{\"first\":2}}","path":[],"extensions":{"test":"value","code":"FETCH_ERROR"}}]}"#.as_bytes())
     });
 
     static REDACTED_PRODUCT_RESPONSE: Lazy<Bytes> = Lazy::new(|| {
         Bytes::from_static(
-            r#"{"data":{"topProducts":null},"errors":[{"message":"Subgraph errors redacted"}]}"#
+            r#"{"data":{"topProducts":null},"errors":[{"message":"Subgraph errors redacted","path":[]}]}"#
                 .as_bytes(),
         )
     });
 
     static REDACTED_ACCOUNT_RESPONSE: Lazy<Bytes> = Lazy::new(|| {
         Bytes::from_static(
-            r#"{"data":null,"errors":[{"message":"Subgraph errors redacted"}]}"#.as_bytes(),
+            r#"{"data":null,"errors":[{"message":"Subgraph errors redacted","path":[]}]}"#
+                .as_bytes(),
         )
     });
 

apollo-router/src/query_planner/fetch.rs

@@ -582,6 +582,7 @@ impl FetchNode {
                         errors.push(error);
                     }
                 } else {
+                    error.path = Some(current_dir.clone());
                     errors.push(error);
                 }
             }
@@ -639,13 +640,17 @@ impl FetchNode {
                 .errors
                 .into_iter()
                 .map(|error| {
-                    let path = error.path.as_ref().map(|path| {
-                        Path::from_iter(current_slice.iter().chain(path.iter()).cloned())
-                    });
+                    let path = error
+                        .path
+                        .as_ref()
+                        .map(|path| {
+                            Path::from_iter(current_slice.iter().chain(path.iter()).cloned())
+                        })
+                        .unwrap_or_else(|| current_dir.clone());
 
                     Error {
                         locations: error.locations,
-                        path,
+                        path: Some(path),
                         message: error.message,
                         extensions: error.extensions,
                     }

apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__missing_entities.snap

@@ -1,5 +1,5 @@
 ---
-source: apollo-router/src/services/supergraph_service.rs
+source: apollo-router/src/services/supergraph/tests.rs
 expression: stream.next_response().await.unwrap()
 ---
 {
@@ -14,7 +14,11 @@ expression: stream.next_response().await.unwrap()
   },
   "errors": [
     {
-      "message": "error"
+      "message": "error",
+      "path": [
+        "currentUser",
+        "activeOrganization"
+      ]
     }
   ]
 }

apollo-router/tests/integration/batching.rs

@@ -140,19 +140,20 @@ async fn it_batches_with_errors_in_single_graph() -> Result<(), BoxError> {
     if test_is_enabled() {
         // Make sure that we got back what we wanted
         assert_yaml_snapshot!(responses, @r###"
-    ---
-    - data:
-        entryA:
-          index: 0
-    - errors:
-        - message: expected error in A
-    - data:
-        entryA:
-          index: 2
-    - data:
-        entryA:
-          index: 3
-    "###);
+        ---
+        - data:
+            entryA:
+              index: 0
+        - errors:
+            - message: expected error in A
+              path: []
+        - data:
+            entryA:
+              index: 2
+        - data:
+            entryA:
+              index: 3
+        "###);
     }
 
     Ok(())
@@ -189,24 +190,26 @@ async fn it_batches_with_errors_in_multi_graph() -> Result<(), BoxError> {
 
     if test_is_enabled() {
         assert_yaml_snapshot!(responses, @r###"
-    ---
-    - data:
-        entryA:
-          index: 0
-    - data:
-        entryB:
-          index: 0
-    - errors:
-        - message: expected error in A
-    - errors:
-        - message: expected error in B
-    - data:
-        entryA:
-          index: 2
-    - data:
-        entryB:
-          index: 2
-    "###);
+        ---
+        - data:
+            entryA:
+              index: 0
+        - data:
+            entryB:
+              index: 0
+        - errors:
+            - message: expected error in A
+              path: []
+        - errors:
+            - message: expected error in B
+              path: []
+        - data:
+            entryA:
+              index: 2
+        - data:
+            entryB:
+              index: 2
+        "###);
     }
 
     Ok(())
@@ -250,13 +253,15 @@ async fn it_handles_short_timeouts() -> Result<(), BoxError> {
               index: 0
         - errors:
             - message: Request timed out
+              path: []
               extensions:
                 code: REQUEST_TIMEOUT
         - data:
             entryA:
               index: 1
         - errors:
             - message: Request timed out
+              path: []
               extensions:
                 code: REQUEST_TIMEOUT
         "###);
@@ -323,14 +328,17 @@ async fn it_handles_indefinite_timeouts() -> Result<(), BoxError> {
               index: 2
         - errors:
             - message: Request timed out
+              path: []
               extensions:
                 code: REQUEST_TIMEOUT
         - errors:
             - message: Request timed out
+              path: []
               extensions:
                 code: REQUEST_TIMEOUT
         - errors:
             - message: Request timed out
+              path: []
               extensions:
                 code: REQUEST_TIMEOUT
         "###);
@@ -554,22 +562,24 @@ async fn it_handles_cancelled_by_coprocessor() -> Result<(), BoxError> {
 
     if test_is_enabled() {
         assert_yaml_snapshot!(responses, @r###"
-    ---
-    - errors:
-        - message: Subgraph A is not allowed
-          extensions:
-            code: ERR_NOT_ALLOWED
-    - data:
-        entryB:
-          index: 0
-    - errors:
-        - message: Subgraph A is not allowed
-          extensions:
-            code: ERR_NOT_ALLOWED
-    - data:
-        entryB:
-          index: 1
-    "###);
+        ---
+        - errors:
+            - message: Subgraph A is not allowed
+              path: []
+              extensions:
+                code: ERR_NOT_ALLOWED
+        - data:
+            entryB:
+              index: 0
+        - errors:
+            - message: Subgraph A is not allowed
+              path: []
+              extensions:
+                code: ERR_NOT_ALLOWED
+        - data:
+            entryB:
+              index: 1
+        "###);
     }
 
     Ok(())
@@ -697,33 +707,34 @@ async fn it_handles_single_request_cancelled_by_coprocessor() -> Result<(), BoxE
 
     if test_is_enabled() {
         assert_yaml_snapshot!(responses, @r###"
-    ---
-    - data:
-        entryA:
-          index: 0
-    - data:
-        entryB:
-          index: 0
-    - data:
-        entryA:
-          index: 1
-    - data:
-        entryB:
-          index: 1
-    - errors:
-        - message: Subgraph A index 2 is not allowed
-          extensions:
-            code: ERR_NOT_ALLOWED
-    - data:
-        entryB:
-          index: 2
-    - data:
-        entryA:
-          index: 3
-    - data:
-        entryB:
-          index: 3
-    "###);
+        ---
+        - data:
+            entryA:
+              index: 0
+        - data:
+            entryB:
+              index: 0
+        - data:
+            entryA:
+              index: 1
+        - data:
+            entryB:
+              index: 1
+        - errors:
+            - message: Subgraph A index 2 is not allowed
+              path: []
+              extensions:
+                code: ERR_NOT_ALLOWED
+        - data:
+            entryB:
+              index: 2
+        - data:
+            entryA:
+              index: 3
+        - data:
+            entryB:
+              index: 3
+        "###);
     }
 
     Ok(())

apollo-router/tests/integration/snapshots/integration_tests__integration__traffic_shaping__subgraph_rate_limit-2.snap

@@ -2,4 +2,4 @@
 source: apollo-router/tests/integration/traffic_shaping.rs
 expression: response
 ---
-"{\"data\":null,\"errors\":[{\"message\":\"Your request has been rate limited\",\"extensions\":{\"code\":\"REQUEST_RATE_LIMITED\"}}]}"
+"{\"data\":null,\"errors\":[{\"message\":\"Your request has been rate limited\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_RATE_LIMITED\"}}]}"

apollo-router/tests/integration/snapshots/integration_tests__integration__traffic_shaping__subgraph_timeout.snap

@@ -1,5 +1,5 @@
 ---
 source: apollo-router/tests/integration/traffic_shaping.rs
-expression: response.text().await?
+expression: response
 ---
-"{\"data\":null,\"errors\":[{\"message\":\"Request timed out\",\"extensions\":{\"code\":\"REQUEST_TIMEOUT\"}}]}"
+"{\"data\":null,\"errors\":[{\"message\":\"Request timed out\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_TIMEOUT\"}}]}"

apollo-router/tests/integration/subgraph_response.rs

@@ -118,6 +118,7 @@ async fn test_invalid_error_locations() -> Result<(), BoxError> {
             "data": null,
             "errors": [{
                 "message":"service 'products' response was malformed: invalid `locations` within error: invalid type: boolean `true`, expected u32",
+                "path": [],
                 "extensions": {
                     "service": "products",
                     "reason": "invalid `locations` within error: invalid type: boolean `true`, expected u32",