diff --git a/apollo-federation-types/src/build/error.rs b/apollo-federation-types/src/build/error.rs index 642e4b5bd..f61fb7120 100644 --- a/apollo-federation-types/src/build/error.rs +++ b/apollo-federation-types/src/build/error.rs @@ -6,6 +6,7 @@ use std::{ use serde::{Deserialize, Serialize}; #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] pub struct BuildError { /// A message describing the build error. message: Option, @@ -21,6 +22,8 @@ pub struct BuildError { other: crate::UncaughtJson, nodes: Option>, + + omitted_nodes_count: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -87,12 +90,19 @@ impl BuildError { code: Option, message: Option, nodes: Option>, + omitted_nodes_count: Option, ) -> BuildError { - BuildError::new(code, message, BuildErrorType::Composition, nodes) + BuildError::new( + code, + message, + BuildErrorType::Composition, + nodes, + omitted_nodes_count, + ) } pub fn config_error(code: Option, message: Option) -> BuildError { - BuildError::new(code, message, BuildErrorType::Config, None) + BuildError::new(code, message, BuildErrorType::Config, None, None) } fn new( @@ -100,6 +110,7 @@ impl BuildError { message: Option, r#type: BuildErrorType, nodes: Option>, + omitted_nodes_count: Option, ) -> BuildError { let real_message = if code.is_none() && message.is_none() { Some("An unknown error occurred during the build.".to_string()) @@ -112,6 +123,7 @@ impl BuildError { r#type, other: crate::UncaughtJson::new(), nodes, + omitted_nodes_count, } } @@ -130,6 +142,10 @@ impl BuildError { pub fn get_nodes(&self) -> Option> { self.nodes.clone() } + + pub fn get_omitted_nodes_count(&self) -> Option { + self.omitted_nodes_count + } } #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] @@ -264,13 +280,19 @@ impl Error for BuildErrors {} mod tests { use super::{BuildError, BuildErrors}; + use crate::build::BuildErrorNode; use serde_json::{json, Value}; #[test] fn it_supports_iter() { let build_errors: BuildErrors = vec![ - BuildError::composition_error(None, Some("wow".to_string()), None), - BuildError::composition_error(Some("BOO".to_string()), Some("boo".to_string()), None), + BuildError::composition_error(None, Some("wow".to_string()), None, None), + BuildError::composition_error( + Some("BOO".to_string()), + Some("boo".to_string()), + None, + None, + ), ] .into(); @@ -293,9 +315,26 @@ mod tests { #[test] fn it_can_serialize_some_build_errors() { + let error_node = BuildErrorNode { + subgraph: Some("foo".to_string()), + source: None, + start: None, + end: None, + }; + let build_errors: BuildErrors = vec![ - BuildError::composition_error(None, Some("wow".to_string()), None), - BuildError::composition_error(Some("BOO".to_string()), Some("boo".to_string()), None), + BuildError::composition_error( + None, + Some("wow".to_string()), + Some(vec![error_node.clone()]), + Some(1), + ), + BuildError::composition_error( + Some("BOO".to_string()), + Some("boo".to_string()), + Some(vec![error_node.clone()]), + Some(2), + ), ] .into(); @@ -311,16 +350,44 @@ mod tests { "message": "wow", "code": null, "type": "composition", - "nodes": null + "nodes": [ + { + "subgraph": "foo", + "source": null, + "start": null, + "end": null + } + ], + "omittedNodesCount": 1 }, { "message": "boo", "code": "BOO", "type": "composition", - "nodes": null + "nodes": [ + { + "subgraph": "foo", + "source": null, + "start": null, + "end": null + } + ], + "omittedNodesCount": 2 } ] }); assert_eq!(actual_value, expected_value); } + + #[test] + fn it_can_deserialize() { + let msg = "wow".to_string(); + let code = "boo".to_string(); + let actual_struct = serde_json::from_str( + &json!({ "message": &msg, "code": &code, "type": "composition", "nodes": null, "omittedNodesCount": 12 }).to_string(), + ).unwrap(); + let expected_struct = + BuildError::composition_error(Some(code.clone()), Some(msg.clone()), None, Some(12)); + assert_eq!(expected_struct, actual_struct); + } } diff --git a/apollo-federation-types/src/build/hint.rs b/apollo-federation-types/src/build/hint.rs index 460bbc3f5..7d4b789b0 100644 --- a/apollo-federation-types/src/build/hint.rs +++ b/apollo-federation-types/src/build/hint.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; /// New fields added to this struct must be optional in order to maintain /// backwards compatibility with old versions of Rover. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] pub struct BuildHint { /// The message of the hint pub message: String, @@ -14,17 +15,25 @@ pub struct BuildHint { pub nodes: Option>, + pub omitted_nodes_count: Option, + /// Other untyped JSON included in the build hint. #[serde(flatten)] pub other: crate::UncaughtJson, } impl BuildHint { - pub fn new(message: String, code: String, nodes: Option>) -> Self { + pub fn new( + message: String, + code: String, + nodes: Option>, + omitted_nodes_count: Option, + ) -> Self { Self { message, code: Some(code), nodes, + omitted_nodes_count, other: crate::UncaughtJson::new(), } } @@ -40,8 +49,9 @@ mod tests { fn it_can_serialize() { let msg = "hint".to_string(); let code = "hintCode".to_string(); - let expected_json = json!({ "message": &msg, "code": &code, "nodes": null }); - let actual_json = serde_json::to_value(&BuildHint::new(msg, code, None)).unwrap(); + let expected_json = + json!({ "message": &msg, "code": &code, "nodes": null, "omittedNodesCount": null }); + let actual_json = serde_json::to_value(&BuildHint::new(msg, code, None, None)).unwrap(); assert_eq!(expected_json, actual_json) } @@ -50,10 +60,11 @@ mod tests { let msg = "hint".to_string(); let code = "hintCode".to_string(); let actual_struct = serde_json::from_str( - &json!({ "message": &msg, "code": &code, "nodes": null }).to_string(), + &json!({ "message": &msg, "code": &code, "nodes": null, "omittedNodesCount": 12 }) + .to_string(), ) .unwrap(); - let expected_struct = BuildHint::new(msg, code, None); + let expected_struct = BuildHint::new(msg, code, None, Some(12)); assert_eq!(expected_struct, actual_struct); } @@ -68,7 +79,7 @@ mod tests { .to_string(), ) .unwrap(); - let mut expected_struct = BuildHint::new(msg, code, None); + let mut expected_struct = BuildHint::new(msg, code, None, None); expected_struct .other .insert(unexpected_key, Value::String(unexpected_value)); diff --git a/apollo-federation-types/src/build/output.rs b/apollo-federation-types/src/build/output.rs index 77c31027f..4a1b62edf 100644 --- a/apollo-federation-types/src/build/output.rs +++ b/apollo-federation-types/src/build/output.rs @@ -56,12 +56,12 @@ mod tests { let hint_two = "hint-two".to_string(); let code = "code".to_string(); let code2 = "code2".to_string(); - let expected_json = json!({"supergraphSdl": &sdl, "hints": [{"message": &hint_one, "code": &code, "nodes": null}, {"message": &hint_two, "code": &code2, "nodes": null}]}); + let expected_json = json!({"supergraphSdl": &sdl, "hints": [{"message": &hint_one, "code": &code, "nodes": null, "omittedNodesCount": null}, {"message": &hint_two, "code": &code2, "nodes": null, "omittedNodesCount": null}]}); let actual_json = serde_json::to_value(&BuildOutput::new_with_hints( sdl.to_string(), vec![ - BuildHint::new(hint_one, code, None), - BuildHint::new(hint_two, code2, None), + BuildHint::new(hint_one, code, None, None), + BuildHint::new(hint_two, code2, None, None), ], )) .unwrap(); @@ -91,8 +91,8 @@ mod tests { let expected_struct = BuildOutput::new_with_hints( sdl, vec![ - BuildHint::new(hint_one, code, None), - BuildHint::new(hint_two, code2, None), + BuildHint::new(hint_one, code, None, None), + BuildHint::new(hint_two, code2, None, None), ], ); diff --git a/federation-1/harmonizer/src/js_types.rs b/federation-1/harmonizer/src/js_types.rs index 61d8f6278..7246b9b47 100644 --- a/federation-1/harmonizer/src/js_types.rs +++ b/federation-1/harmonizer/src/js_types.rs @@ -59,7 +59,7 @@ impl Display for CompositionError { impl From for BuildError { fn from(input: CompositionError) -> Self { - Self::composition_error(input.code, input.message, input.nodes) + Self::composition_error(input.code, input.message, input.nodes, None) } } diff --git a/federation-2/harmonizer/js-src/composition.ts b/federation-2/harmonizer/js-src/composition.ts index be59ba2f9..7a1a46c55 100644 --- a/federation-2/harmonizer/js-src/composition.ts +++ b/federation-2/harmonizer/js-src/composition.ts @@ -9,9 +9,13 @@ import { } from "./types"; import { ERRORS } from "@apollo/federation-internals"; +const DEFAULT_NODES_SIZE_LIMIT: number = Number.MAX_VALUE; + export function composition( - serviceList: { sdl: string; name: string; url?: string }[] + serviceList: { sdl: string; name: string; url?: string }[], + nodesLimit?: number | null ): CompositionResult { + let limit = nodesLimit || DEFAULT_NODES_SIZE_LIMIT; if (!serviceList || !Array.isArray(serviceList)) { throw new Error("Error in JS-Rust-land: serviceList missing or incorrect."); } @@ -38,12 +42,25 @@ export function composition( if (composed.hints) { composed.hints.map((composed_hint) => { let nodes: BuildErrorNode[] = []; - composed_hint.nodes?.map((node) => nodes.push(getBuildErrorNode(node))); + + let omittedNodesCount = 0; + // for issues that happen in all subgraphs and with a large amount of subgraphs, + // only add nodes up to the limit to prevent massive responses + // (OOM errors when going from js to rust) + if (composed_hint.nodes?.length >= limit) { + composed_hint.nodes + ?.slice(0, limit) + .map((node) => nodes.push(getBuildErrorNode(node))); + omittedNodesCount = composed_hint.nodes?.length - limit; + } else { + composed_hint.nodes?.map((node) => nodes.push(getBuildErrorNode(node))); + } hints.push({ message: composed_hint.toString(), code: composed_hint.definition.code, nodes, + omittedNodesCount: omittedNodesCount, }); }); } @@ -53,12 +70,25 @@ export function composition( let errors: CompositionError[] = []; composed.errors.map((err) => { let nodes: BuildErrorNode[] = []; - err.nodes?.map((node) => nodes.push(getBuildErrorNode(node))); + + let omittedNodesCount = 0; + // for issues that happen in all subgraphs and with a large amount of subgraphs, + // only add nodes up to the limit to prevent massive responses + // (OOM errors when going from js to rust) + if (err.nodes?.length >= limit) { + err.nodes + ?.slice(0, limit) + .map((node) => nodes.push(getBuildErrorNode(node))); + omittedNodesCount = err.nodes?.length - limit; + } else { + err.nodes?.map((node) => nodes.push(getBuildErrorNode(node))); + } errors.push({ code: (err?.extensions["code"] as string) ?? "", message: err.message, nodes, + omittedNodesCount: omittedNodesCount, }); }); diff --git a/federation-2/harmonizer/js-src/do_compose.ts b/federation-2/harmonizer/js-src/do_compose.ts index 8daa97bd6..9e40e447e 100644 --- a/federation-2/harmonizer/js-src/do_compose.ts +++ b/federation-2/harmonizer/js-src/do_compose.ts @@ -10,12 +10,13 @@ declare let composition_bridge: { composition: typeof composition }; declare let done: (compositionResult: CompositionResult) => void; declare let serviceList: { sdl: string; name: string; url?: string }[]; +declare let nodesLimit: number | null; try { // /** // * @type {{ errors: Error[], supergraphSdl?: undefined, hints: undefined } | { errors?: undefined, supergraphSdl: string, hints: string }} // */ - const composed = composition_bridge.composition(serviceList); + const composed = composition_bridge.composition(serviceList, nodesLimit); done(composed); } catch (err) { diff --git a/federation-2/harmonizer/js-src/types.ts b/federation-2/harmonizer/js-src/types.ts index 75e835162..66f8f5f9a 100644 --- a/federation-2/harmonizer/js-src/types.ts +++ b/federation-2/harmonizer/js-src/types.ts @@ -6,6 +6,7 @@ export type CompositionError = { message?: string; code?: string; nodes: BuildErrorNode[]; + omittedNodesCount: number; }; export type BuildErrorNode = { @@ -26,4 +27,5 @@ export type CompositionHint = { message: string; code: string; nodes: BuildErrorNode[]; + omittedNodesCount: number; }; diff --git a/federation-2/harmonizer/src/js_types.rs b/federation-2/harmonizer/src/js_types.rs index 2e3a61f1c..af5848064 100644 --- a/federation-2/harmonizer/src/js_types.rs +++ b/federation-2/harmonizer/src/js_types.rs @@ -18,6 +18,7 @@ use apollo_federation_types::build::{BuildError, BuildErrorNode}; /// [`graphql-js']: https://npm.im/graphql /// [`GraphQLError`]: https://github.com/graphql/graphql-js/blob/3869211/src/error/GraphQLError.js#L18-L75 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] pub(crate) struct CompositionError { /// A human-readable description of the error that prevented composition. message: Option, @@ -29,6 +30,8 @@ pub(crate) struct CompositionError { code: Option, nodes: Option>, + + omitted_nodes_count: Option, } impl CompositionError { @@ -38,6 +41,7 @@ impl CompositionError { extensions: None, code: None, nodes: None, + omitted_nodes_count: Some(u32::default()), } } } @@ -59,7 +63,12 @@ impl Display for CompositionError { impl From for BuildError { fn from(input: CompositionError) -> Self { - Self::composition_error(input.code, input.message, input.nodes) + Self::composition_error( + input.code, + input.message, + input.nodes, + input.omitted_nodes_count, + ) } } diff --git a/federation-2/harmonizer/src/lib.rs b/federation-2/harmonizer/src/lib.rs index 40174950a..d0bab7edb 100644 --- a/federation-2/harmonizer/src/lib.rs +++ b/federation-2/harmonizer/src/lib.rs @@ -44,6 +44,16 @@ use apollo_federation_types::build::{ /// The `harmonize` function receives a [`Vec`] and invokes JavaScript /// composition on it, either returning the successful output, or a list of error messages. pub fn harmonize(subgraph_definitions: Vec) -> BuildResult { + harmonize_limit(subgraph_definitions, None) +} + +/// The `harmonize` function receives a [`Vec`] and invokes JavaScript +/// composition on it, either returning the successful output, or a list of error messages. +/// `nodes_limit` limits the number of returns schema nodes to prevent OOM issues +pub fn harmonize_limit( + subgraph_definitions: Vec, + nodes_limit: Option, +) -> BuildResult { // The snapshot is created in the build_harmonizer.rs script and included in our binary image let buffer = include_bytes!(concat!(env!("OUT_DIR"), "/composition.snap")); @@ -82,6 +92,22 @@ pub fn harmonize(subgraph_definitions: Vec) -> BuildResult { ) .expect("unable to evaluate service list in JavaScript runtime"); + // store the nodes_limit variable in the nodesLimit variable + runtime + .execute_script( + "", + deno_core::FastString::Owned( + format!( + "nodesLimit = {}", + nodes_limit + .map(|n| n.to_string()) + .unwrap_or("null".to_string()) + ) + .into(), + ), + ) + .expect("unable to evaluate nodes limit in JavaScript runtime"); + // run the unmodified do_compose.js file, which expects `serviceList` to be set runtime .execute_script(