add a test which panics in a mock subgraph and handles the response

apollo-router-core/src/layers/apq.rs

@@ -244,7 +244,6 @@ mod apq_tests {
     async fn it_doesnt_update_the_cache_if_the_hash_is_not_valid() {
         let hash = Cow::from("ecf4edb46db40b5132295c0291d62fb65d6759a9eedfa4d5d612dd5ec54a6b36");
         let hash2 = hash.clone();
-        let hash3 = hash.clone();
 
         let expected_apq_miss_error = crate::Error {
             message: "PersistedQueryNotFound".to_string(),
@@ -285,34 +284,9 @@ mod apq_tests {
                     .build()
                     .expect("expecting valid request"))
             });
-        mock_service_builder
-            // the second last one should have the right APQ header and the full query string
-            // even though the query string wasn't provided by the client
-            .expect_call()
-            .times(1)
-            .returning(move |req| {
-                let body = req.originating_request.body();
 
-                let as_json = body.extensions.get("persistedQuery").unwrap();
-
-                let persisted_query: PersistedQuery =
-                    serde_json_bytes::from_value(as_json.clone()).unwrap();
-
-                assert_eq!(persisted_query.sha256hash, hash3);
-
-                assert!(body.query.is_some());
-
-                let hash = hex::decode(hash3.as_bytes()).unwrap();
-
-                assert!(!query_matches_hash(
-                    body.query.clone().unwrap_or_default().as_str(),
-                    hash.as_slice()
-                ));
-
-                Ok(RouterResponse::fake_builder()
-                    .build()
-                    .expect("expecting valid request"))
-            });
+        // the last call should be an APQ error.
+        // the provided hash was wrong, so the query wasn't inserted into the cache.
 
         let mock_service = mock_service_builder.build();
 

apollo-router-core/src/query_planner/mod.rs

@@ -594,6 +594,46 @@ mod tests {
         );
     }
 
+    /// This test panics in the product subgraph. HOWEVER, this does not result in a panic in the
+    /// test, since the buffer() functionality in the tower stack "loses" the panic and we end up
+    /// with a closed service.
+    ///
+    /// See: https://github.com/tower-rs/tower/issues/455
+    ///
+    /// The query planner reports the failed subgraph fetch as an error with a reason of "service
+    /// closed", which is what this test expects.
+    #[tokio::test]
+    async fn mock_subgraph_service_withf_panics_should_be_reported_as_service_closed() {
+        let query_plan: QueryPlan = QueryPlan {
+            root: serde_json::from_str(test_query_plan!()).unwrap(),
+        };
+
+        let mut mock_products_service = plugin::utils::test::MockSubgraphService::new();
+        mock_products_service.expect_call().times(1).withf(|_| {
+            panic!("this panic should be propagated to the test harness");
+        });
+
+        let result = query_plan
+            .execute(
+                &Context::new(),
+                &ServiceRegistry::new(HashMap::from([(
+                    "product".into(),
+                    ServiceBuilder::new()
+                        .buffer(1)
+                        .service(mock_products_service.build().boxed()),
+                )])),
+                http_compat::Request::mock(),
+                &Schema::from_str(test_schema!()).unwrap(),
+            )
+            .await;
+        assert_eq!(result.errors.len(), 1);
+        let reason: String = serde_json_bytes::from_value(
+            result.errors[0].extensions.get("reason").unwrap().clone(),
+        )
+        .unwrap();
+        assert_eq!(reason, "service closed".to_string());
+    }
+
     #[tokio::test]
     async fn fetch_includes_operation_name() {
         let query_plan: QueryPlan = QueryPlan {
@@ -612,7 +652,8 @@ mod tests {
                     == Some("topProducts_product_0".into());
                 inner_succeeded.store(matches, Ordering::SeqCst);
                 matches
-            });
+            })
+            .returning(|_| Ok(SubgraphResponse::fake_builder().build()));
         query_plan
             .execute(
                 &Context::new(),
@@ -647,7 +688,8 @@ mod tests {
                 let matches = request.subgraph_request.method() == Method::POST;
                 inner_succeeded.store(matches, Ordering::SeqCst);
                 matches
-            });
+            })
+            .returning(|_| Ok(SubgraphResponse::fake_builder().build()));
         query_plan
             .execute(
                 &Context::new(),