Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions apollo-router/src/axum_factory/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use tower::BoxError;
use tower::Service;
use tower::ServiceExt;
use tower::service_fn;
use uuid::Uuid;

use super::*;
use crate::ApolloRouterError;
Expand Down Expand Up @@ -1047,7 +1048,7 @@ async fn response_failure() -> Result<(), ApolloRouterError> {
.await;
let (server, client) = init(router_service).await;

let response = client
let mut response = client
.post(format!(
"{}/",
server.graphql_listen_address().as_ref().unwrap()
Expand All @@ -1066,15 +1067,17 @@ async fn response_failure() -> Result<(), ApolloRouterError> {
.await
.unwrap();

assert_eq!(
response,
crate::error::FetchError::SubrequestHttpError {
status_code: Some(200),
service: "Mock service".to_string(),
reason: "Mock error".to_string(),
}
.to_response()
);
let mut expected_response = crate::error::FetchError::SubrequestHttpError {
status_code: Some(200),
service: "Mock service".to_string(),
reason: "Mock error".to_string(),
}
.to_response();
// Overwrite error IDs to avoid random Uuid mismatch
Comment thread
rregitsky marked this conversation as resolved.
Outdated
response.errors[0].set_apollo_id(Uuid::nil());
expected_response.errors[0].set_apollo_id(Uuid::nil());

assert_eq!(response, expected_response);
server.shutdown().await
}

Expand Down
17 changes: 10 additions & 7 deletions apollo-router/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ impl FetchError {
}
}

Error {
message: self.to_string(),
locations: Default::default(),
path,
extensions: value.as_object().unwrap().to_owned(),
}
Error::builder()
.message(self.to_string())
.locations(Vec::default())
.and_path(path)
.extensions(value.as_object().unwrap().to_owned())
.build()
}

/// Convert the error to an appropriate response.
Expand Down Expand Up @@ -660,6 +660,9 @@ mod tests {
)
.build();

assert_eq!(expected_gql_error, error.to_graphql_error(None));
assert_eq!(
expected_gql_error.with_null_id(),
error.to_graphql_error(None).with_null_id()
);
}
}
100 changes: 83 additions & 17 deletions apollo-router/src/graphql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod visitor;

use std::fmt;
use std::pin::Pin;
use std::str::FromStr;

use apollo_compiler::response::GraphQLError as CompilerExecutionError;
use apollo_compiler::response::ResponseDataPathSegment;
Expand All @@ -20,6 +21,7 @@ use serde::Serialize;
use serde_json_bytes::ByteString;
use serde_json_bytes::Map as JsonMap;
use serde_json_bytes::Value;
use uuid::Uuid;
pub(crate) use visitor::ResponseVisitor;

use crate::json_ext::Object;
Expand Down Expand Up @@ -52,7 +54,7 @@ pub struct Location {
/// as may be found in the `errors` field of a GraphQL [`Response`].
///
/// Converted to (or from) JSON with serde.
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, Default)]
Comment thread
timbotnik marked this conversation as resolved.
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
#[non_exhaustive]
pub struct Error {
Expand All @@ -70,6 +72,10 @@ pub struct Error {
/// The optional GraphQL extensions for this error.
#[serde(default, skip_serializing_if = "Object::is_empty")]
pub extensions: Object,

/// A unique identifier for this error
#[serde(default = "generate_uuid", skip_serializing)]
apollo_id: Uuid,
}
// Implement getter and getter_mut to not use pub field directly

Expand Down Expand Up @@ -103,25 +109,39 @@ impl Error {
/// Optional, may be called multiple times.
/// Adds one item to the [`Error::extensions`] map.
///
/// * `.extension_code(impl Into<`[`String`]`>)`
/// Optional.
/// Sets the "code" in the extension map. Will be ignored if extension already has this key
/// set.
///
/// * `.apollo_id(impl Into<`[`Uuid`]`>)`
/// Optional.
/// Sets the unique identifier for this Error. This should only be used in cases of
/// deserialization or testing. If not given, the ID will be auto-generated.
///
/// * `.build()`
/// Finishes the builder and returns a GraphQL [`Error`].
#[builder(visibility = "pub")]
fn new<T: Into<String>>(
fn new(
message: String,
locations: Vec<Location>,
path: Option<Path>,
extension_code: T,
extension_code: Option<String>,
// Skip the `Object` type alias in order to use buildstructor’s map special-casing
mut extensions: JsonMap<ByteString, Value>,
apollo_id: Option<Uuid>,
) -> Self {
extensions
.entry("code")
.or_insert_with(|| extension_code.into().into());
if let Some(code) = extension_code {
extensions
.entry("code")
.or_insert(Value::String(ByteString::from(code)));
}
Self {
message,
locations,
path,
extensions,
apollo_id: apollo_id.unwrap_or_else(Uuid::new_v4),
}
}

Expand Down Expand Up @@ -160,13 +180,24 @@ impl Error {
.map_err(|err| MalformedResponseError {
reason: format!("invalid `path` within error: {}", err),
})?;

Ok(Error {
message,
locations,
path,
extensions,
let apollo_id: Option<Uuid> = extract_key_value_from_object!(
object,
"apolloId",
Value::String(s) => s
)
.map_err(|err| MalformedResponseError {
reason: format!("invalid `apolloId` within error: {}", err),
})?
.map(|s| {
Uuid::from_str(s.as_str()).map_err(|err| MalformedResponseError {
reason: format!("invalid `apolloId` within error: {}", err),
})
})
.transpose()?;

Ok(Self::new(
message, locations, path, None, extensions, apollo_id,
))
}

pub(crate) fn from_value_completion_value(value: &Value) -> Option<Error> {
Expand Down Expand Up @@ -196,13 +227,47 @@ impl Error {
.and_then(|p: &serde_json_bytes::Value| -> Option<Path> {
serde_json_bytes::from_value(p.clone()).ok()
});
Some(Error {
message,
locations,
path,
extensions,

Some(Self::new(
message, locations, path, None, extensions,
None, // apollo_id is not serialized, so it will never exist in a serialized vc error
))
}

/// Extract the error code from [`Error::extensions`] as a String if it is set.
pub fn extension_code(&self) -> Option<String> {
self.extensions.get("code").and_then(|c| match c {
Value::String(s) => Some(s.as_str().to_owned()),
Value::Bool(b) => Some(format!("{b}")),
Comment thread
rregitsky marked this conversation as resolved.
Outdated
Value::Number(n) => Some(n.to_string()),
Value::Null | Value::Array(_) | Value::Object(_) => None,
})
}

/// Retrieve the internal Apollo unique ID for this error
pub fn apollo_id(&self) -> Uuid {
self.apollo_id
}

#[cfg(test)]
/// Mutates the [`self.apollo_id`] to `Uuid::nil()` for comparing errors in tests where you
/// cannot extract the randomly generated Uuid
pub fn with_null_id(mut self) -> Self {
self.set_apollo_id(Uuid::nil());
self
}

#[cfg(test)]
/// Updates [`self.apollo_id`] to `id`.
pub fn set_apollo_id(&mut self, id: Uuid) {
self.apollo_id = id;
}
}

/// Generate a random Uuid. For use in generating a default [`Error:apollo_id`] when not supplied
/// during deserialization.
fn generate_uuid() -> Uuid {
Uuid::new_v4()
}

/// GraphQL spec require that both "line" and "column" are positive numbers.
Expand Down Expand Up @@ -282,6 +347,7 @@ impl From<CompilerExecutionError> for Error {
locations,
path,
extensions,
apollo_id: Uuid::new_v4(),
}
}
}
97 changes: 55 additions & 42 deletions apollo-router/src/graphql/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,34 +258,37 @@ impl From<ExecutionResponse> for Response {
mod tests {
use serde_json::json;
use serde_json_bytes::json as bjson;
use uuid::Uuid;

use super::*;
use crate::graphql::Location;

#[test]
fn test_append_errors_path_fallback_and_override() {
let uuid1 = Uuid::new_v4();
let uuid2 = Uuid::new_v4();
let expected_errors = vec![
Error {
message: "Something terrible happened!".to_string(),
path: Some(Path::from("here")),
..Default::default()
},
Error {
message: "I mean for real".to_string(),
..Default::default()
},
Error::builder()
.message("Something terrible happened!")
.path(Path::from("here"))
.apollo_id(uuid1)
.build(),
Error::builder()
.message("I mean for real")
.apollo_id(uuid2)
.build(),
];

let mut errors_to_append = vec![
Error {
message: "Something terrible happened!".to_string(),
path: Some(Path::from("here")),
..Default::default()
},
Error {
message: "I mean for real".to_string(),
..Default::default()
},
Error::builder()
.message("Something terrible happened!")
.path(Path::from("here"))
.apollo_id(uuid1)
.build(),
Error::builder()
.message("I mean for real")
.apollo_id(uuid2)
.build(),
];

let mut response = Response::builder().build();
Expand Down Expand Up @@ -334,8 +337,9 @@ mod tests {
.to_string()
.as_str(),
);
let response = result.unwrap();
assert_eq!(
result.unwrap(),
response,
Response::builder()
.data(json!({
"hero": {
Expand All @@ -356,17 +360,21 @@ mod tests {
]
}
}))
.errors(vec![Error {
message: "Name for character with ID 1002 could not be fetched.".into(),
locations: vec!(Location { line: 6, column: 7 }),
path: Some(Path::from("hero/heroFriends/1/name")),
extensions: bjson!({
"error-extension": 5,
})
.as_object()
.cloned()
.unwrap()
}])
.errors(vec![
Error::builder()
.message("Name for character with ID 1002 could not be fetched.")
.locations(vec!(Location { line: 6, column: 7 }))
.path(Path::from("hero/heroFriends/1/name"))
.extensions(
bjson!({ "error-extension": 5, })
.as_object()
.cloned()
.unwrap()
)
// Overwrite ID to avoid random Uuid mismatch
.apollo_id(response.errors[0].apollo_id)
.build()
])
.extensions(
bjson!({
"response-extension": 3,
Expand Down Expand Up @@ -423,8 +431,9 @@ mod tests {
.to_string()
.as_str(),
);
let response = result.unwrap();
assert_eq!(
result.unwrap(),
response,
Response::builder()
.label("part".to_owned())
.data(json!({
Expand All @@ -447,17 +456,21 @@ mod tests {
}
}))
.path(Path::from("hero/heroFriends/1/name"))
.errors(vec![Error {
message: "Name for character with ID 1002 could not be fetched.".into(),
locations: vec!(Location { line: 6, column: 7 }),
path: Some(Path::from("hero/heroFriends/1/name")),
extensions: bjson!({
"error-extension": 5,
})
.as_object()
.cloned()
.unwrap()
}])
.errors(vec![
Error::builder()
.message("Name for character with ID 1002 could not be fetched.")
.locations(vec!(Location { line: 6, column: 7 }))
.path(Path::from("hero/heroFriends/1/name"))
.extensions(
bjson!({ "error-extension": 5, })
.as_object()
.cloned()
.unwrap()
)
// Overwrite ID to avoid random Uuid mismatch
.apollo_id(response.errors[0].apollo_id)
.build()
])
.extensions(
bjson!({
"response-extension": 3,
Expand Down
Loading