From 3f5e724d277efac4d61557e48af810b8507a7eca Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 15 Dec 2022 15:57:52 +0100 Subject: [PATCH 1/3] keep the error path when redacting subgraph errors the path is used to decide whether to assign the error on the primary or deferred response --- .../src/plugins/include_subgraph_errors.rs | 17 ++++++----------- apollo-router/src/spec/query.rs | 4 ---- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/apollo-router/src/plugins/include_subgraph_errors.rs b/apollo-router/src/plugins/include_subgraph_errors.rs index e17cfff077..702553952b 100644 --- a/apollo-router/src/plugins/include_subgraph_errors.rs +++ b/apollo-router/src/plugins/include_subgraph_errors.rs @@ -1,26 +1,18 @@ use std::collections::HashMap; -use once_cell::sync::Lazy; use schemars::JsonSchema; use serde::Deserialize; use tower::BoxError; use tower::ServiceExt; -use crate::error::Error as SubgraphError; +use crate::json_ext::Object; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::register_plugin; use crate::services::subgraph; use crate::SubgraphResponse; -#[allow(clippy::field_reassign_with_default)] -static REDACTED_ERROR_MESSAGE: Lazy> = Lazy::new(|| { - let mut error: SubgraphError = Default::default(); - - error.message = "Subgraph errors redacted".to_string(); - - vec![error] -}); +static REDACTED_ERROR_MESSAGE: &str = "Subgraph errors redacted"; register_plugin!("apollo", "include_subgraph_errors", IncludeSubgraphErrors); @@ -57,7 +49,10 @@ impl Plugin for IncludeSubgraphErrors { .map_response(move |mut response: SubgraphResponse| { if !response.response.body().errors.is_empty() { tracing::info!("redacted subgraph({sub_name_response}) errors"); - response.response.body_mut().errors = REDACTED_ERROR_MESSAGE.clone(); + for error in response.response.body_mut().errors.iter_mut() { + error.message = REDACTED_ERROR_MESSAGE.to_string(); + error.extensions = Object::default(); + } } response }) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 3d08e23ec0..134284814e 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -976,10 +976,6 @@ impl Query { response_path: Option<&Path>, path: &Path, ) -> bool { - println!( - "Query::contains_error_path: path = {path}, query: {}", - self.string, - ); let operation = if let Some(subselection) = subselection { // Get subselection from hashmap match self.subselections.get(&SubSelection { From 67b9723f0685c38e01f578c11ddcdfdf9a256b41 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 16 Dec 2022 10:12:31 +0100 Subject: [PATCH 2/3] changelog --- NEXT_CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 5f7b68663a..83425156c3 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -40,3 +40,12 @@ In addition to the url and poll interval, Uplink poll timeout can now be configu It defaults to 30 seconds. By [@o0Ignition0o](https://github.com/o0Ignition0o) in https://github.com/apollographql/router/pull/2271 + + +## 🐛 Fixes + +### Keep the error path when redacting subgraph errors ([Issue #1818](https://github.com/apollographql/router/issues/1818)) + +Error redaction was erasing the error's path, which made it impossible to affect the errors to deferred responses. Now the redacted errors keep the path. Since the response shape for the primary and deferred responses are defined from the API schema, there is no possibility of leaking internal schema information here. + +By [@Geal](https://github.com/geal) in https://github.com/apollographql/router/pull/2273 \ No newline at end of file From 081e1759c19aec2ffc3a48420858c38f8e093b6b Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 16 Dec 2022 10:18:22 +0100 Subject: [PATCH 3/3] fix test build --- apollo-router/src/plugins/include_subgraph_errors.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/apollo-router/src/plugins/include_subgraph_errors.rs b/apollo-router/src/plugins/include_subgraph_errors.rs index 702553952b..51cdc935af 100644 --- a/apollo-router/src/plugins/include_subgraph_errors.rs +++ b/apollo-router/src/plugins/include_subgraph_errors.rs @@ -77,6 +77,7 @@ mod test { use std::sync::Arc; use bytes::Bytes; + use once_cell::sync::Lazy; use serde_json::Value as jValue; use serde_json_bytes::ByteString; use serde_json_bytes::Value;