From e8d9ef50b7a977ed6cb3f32a089bdc2d97c290ea Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Mon, 5 Sep 2022 09:42:02 +0100 Subject: [PATCH 1/8] slightly better error reporting for unexpected errors This doesn't address the root cause of an unexpected error, but it makes the transtion of the error back through the router a little cleaner. --- .../router-bridge/js-src/plan_worker.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index 9427779b6..211e7d3fa 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -191,10 +191,17 @@ async function run() { } } catch (e) { logger.warn(`an error happened in the worker runtime ${e}\n`); + let unexpectedError = { + name: "unexpectedError", + message: `${e}`, + extensions: { + code: "an error happened in the worker runtime", + }, + }; await send({ id, payload: { - errors: [e], + errors: [unexpectedError], usageReporting: { statsReportKey: "", referencedFieldsByType: {}, @@ -204,9 +211,16 @@ async function run() { } } catch (e) { logger.warn(`plan_worker: an unknown error occured ${e}\n`); + let unexpectedError = { + name: "unexpectedError", + message: `${e}`, + extensions: { + code: "an error happened in the worker runtime", + }, + }; await send({ payload: { - errors: [e], + errors: [unexpectedError], usageReporting: { statsReportKey: "", referencedFieldsByType: {}, From 2d06aef6532a2685bbb5ce72bd0743dea7d13552 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Tue, 6 Sep 2022 11:28:45 +0100 Subject: [PATCH 2/8] try to incorporate review feedback By looking up how typescript works in google and attempting to write some. --- .../router-bridge/js-src/plan_worker.ts | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index 211e7d3fa..defe92ecd 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -191,11 +191,23 @@ async function run() { } } catch (e) { logger.warn(`an error happened in the worker runtime ${e}\n`); + let message = ""; + if (e.hasOwnProperty("message")) { + message = e.message; + } + let name = "unknown"; + if (e.hasOwnProperty("name")) { + name = e.name; + } + let unexpectedError = { - name: "unexpectedError", - message: `${e}`, + name: name, + message: message, extensions: { - code: "an error happened in the worker runtime", + code: "QUERY_PLANNING_FAILED", + exception: { + stacktrace: [e.toString().split(/\n/)], + }, }, }; await send({ @@ -211,11 +223,23 @@ async function run() { } } catch (e) { logger.warn(`plan_worker: an unknown error occured ${e}\n`); + let message = ""; + if (e.hasOwnProperty("message")) { + message = e.message; + } + let name = "unknown"; + if (e.hasOwnProperty("name")) { + name = e.name; + } + let unexpectedError = { - name: "unexpectedError", - message: `${e}`, + name: name, + message: message, extensions: { - code: "an error happened in the worker runtime", + code: "QUERY_PLANNING_FAILED", + exception: { + stacktrace: [e.toString().split(/\n/)], + }, }, }; await send({ From 3f8297970c318734f2d642a4c4d8e17cc1996d78 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 7 Sep 2022 17:53:10 +0200 Subject: [PATCH 3/8] add support of extensions error code Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- federation-2/router-bridge/js-src/plan.ts | 41 +++++++++++++++++-- .../router-bridge/js-src/plan_worker.ts | 28 +++---------- federation-2/router-bridge/src/planner.rs | 26 ++++++++---- 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan.ts b/federation-2/router-bridge/js-src/plan.ts index 61250c7c2..54396c721 100644 --- a/federation-2/router-bridge/js-src/plan.ts +++ b/federation-2/router-bridge/js-src/plan.ts @@ -7,6 +7,7 @@ import { import { DocumentNode, ExecutionResult, + GraphQLError, GraphQLSchema, parse, validate, @@ -25,7 +26,9 @@ import { import { ReferencedFieldsForType } from "apollo-reporting-protobuf"; const PARSE_FAILURE: string = "## GraphQLParseFailure\n"; +const PARSE_FAILURE_EXT_CODE: string = "GRAPHQL_PARSE_FAILED"; const VALIDATION_FAILURE: string = "## GraphQLValidationFailure\n"; +const VALIDATION_FAILURE_EXT_CODE: string = "GRAPHQL_VALIDATION_FAILED"; const UNKNOWN_OPERATION: string = "## GraphQLUnknownOperationName\n"; export type ReferencedFieldsByType = Record; @@ -77,7 +80,14 @@ export class BridgeQueryPlanner { statsReportKey: PARSE_FAILURE, referencedFieldsByType: {}, }, - errors: [parseError], + errors: [ + { + ...parseError, + extensions: { + code: PARSE_FAILURE_EXT_CODE, + }, + }, + ], }; } @@ -90,7 +100,25 @@ export class BridgeQueryPlanner { statsReportKey: VALIDATION_FAILURE, referencedFieldsByType: {}, }, - errors: validationErrors, + errors: validationErrors.map((error): GraphQLError => { + if ( + error.extensions == null || + Object.keys(error.extensions).length === 0 + ) { + return new GraphQLError(error.message, { + extensions: { + code: VALIDATION_FAILURE_EXT_CODE, + }, + path: error.path, + nodes: error.nodes, + originalError: error.originalError, + positions: error.positions, + source: error.source, + }); + } + + return error; + }), }; } @@ -118,7 +146,14 @@ export class BridgeQueryPlanner { statsReportKey, referencedFieldsByType: {}, }, - errors: [e], + errors: [ + { + ...e, + extensions: { + code: VALIDATION_FAILURE_EXT_CODE, + }, + }, + ], }; } diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index defe92ecd..bf4df336d 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -191,18 +191,10 @@ async function run() { } } catch (e) { logger.warn(`an error happened in the worker runtime ${e}\n`); - let message = ""; - if (e.hasOwnProperty("message")) { - message = e.message; - } - let name = "unknown"; - if (e.hasOwnProperty("name")) { - name = e.name; - } - let unexpectedError = { - name: name, - message: message, + const unexpectedError = { + name: e.name || "unknown", + message: e.message || "", extensions: { code: "QUERY_PLANNING_FAILED", exception: { @@ -223,18 +215,10 @@ async function run() { } } catch (e) { logger.warn(`plan_worker: an unknown error occured ${e}\n`); - let message = ""; - if (e.hasOwnProperty("message")) { - message = e.message; - } - let name = "unknown"; - if (e.hasOwnProperty("name")) { - name = e.name; - } - let unexpectedError = { - name: name, - message: message, + const unexpectedError = { + name: e.name || "unknown", + message: e.message || "", extensions: { code: "QUERY_PLANNING_FAILED", exception: { diff --git a/federation-2/router-bridge/src/planner.rs b/federation-2/router-bridge/src/planner.rs index d4eb89f4b..550a3d8fb 100644 --- a/federation-2/router-bridge/src/planner.rs +++ b/federation-2/router-bridge/src/planner.rs @@ -2,14 +2,20 @@ * Instantiate a QueryPlanner from a schema, and perform query planning */ -use crate::worker::JsWorker; -use serde::{de::DeserializeOwned, Deserialize, Serialize}; use std::collections::HashMap; -use std::fmt::{Debug, Display, Formatter}; +use std::fmt::Debug; +use std::fmt::Display; +use std::fmt::Formatter; use std::marker::PhantomData; use std::sync::Arc; + +use serde::de::DeserializeOwned; +use serde::Deserialize; +use serde::Serialize; use thiserror::Error; +use crate::worker::JsWorker; + // ------------------------------------ #[derive(Debug, Serialize)] @@ -50,7 +56,7 @@ pub struct OperationalContext { /// /// [`graphql-js`]: https://npm.im/graphql /// [`GraphQLError`]: https://github.com/graphql/graphql-js/blob/3869211/src/error/GraphQLError.js#L18-L75 -#[derive(Debug, Error, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Error, Serialize, Deserialize, PartialEq, Eq, Clone)] pub struct PlanError { /// A human-readable description of the error that prevented planning. pub message: Option, @@ -108,7 +114,7 @@ impl Display for PlanError { } } -#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)] /// Error codes pub struct PlanErrorExtensions { /// The error code @@ -314,6 +320,7 @@ pub struct PlanErrors { impl std::fmt::Display for PlanErrors { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + dbg!(self); f.write_fmt(format_args!( "query validation errors: {}", self.errors @@ -516,8 +523,10 @@ pub struct IncrementalDeliverySupport { #[cfg(test)] mod tests { + use futures::stream::StreamExt; + use futures::stream::{self}; + use super::*; - use futures::stream::{self, StreamExt}; const QUERY: &str = include_str!("testdata/query.graphql"); const QUERY2: &str = include_str!("testdata/query2.graphql"); @@ -1163,7 +1172,10 @@ GraphQL request:4:9 mod planning_error { use std::collections::HashMap; - use crate::planner::{PlanError, PlanErrorExtensions, ReferencedFieldsForType, UsageReporting}; + use crate::planner::PlanError; + use crate::planner::PlanErrorExtensions; + use crate::planner::ReferencedFieldsForType; + use crate::planner::UsageReporting; #[test] #[should_panic( From 6dcb885533103a72a7f25d1fc307e5783ab47b01 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 8 Sep 2022 14:21:35 +0200 Subject: [PATCH 4/8] more idiomatic code Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../router-bridge/js-src/plan_worker.ts | 20 +++++++++---------- federation-2/router-bridge/src/planner.rs | 10 +++++++++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index bf4df336d..c3e7db3cd 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -1,6 +1,6 @@ import { GraphQLErrorExt } from "@apollo/core-schema/dist/error"; import { QueryPlannerConfig } from "@apollo/query-planner"; -import { ASTNode, Source, SourceLocation } from "graphql"; +import { ASTNode, GraphQLError, Source, SourceLocation } from "graphql"; import { BridgeQueryPlanner, ExecutionResultWithUsageReporting, @@ -192,16 +192,15 @@ async function run() { } catch (e) { logger.warn(`an error happened in the worker runtime ${e}\n`); - const unexpectedError = { - name: e.name || "unknown", - message: e.message || "", + const unexpectedError = new GraphQLError(e.message || "", { extensions: { code: "QUERY_PLANNING_FAILED", exception: { - stacktrace: [e.toString().split(/\n/)], + stacktrace: e.toString().split("\n"), }, }, - }; + }); + unexpectedError.name = e.name || "unknown"; await send({ id, payload: { @@ -216,16 +215,15 @@ async function run() { } catch (e) { logger.warn(`plan_worker: an unknown error occured ${e}\n`); - const unexpectedError = { - name: e.name || "unknown", - message: e.message || "", + const unexpectedError = new GraphQLError(e.message || "", { extensions: { code: "QUERY_PLANNING_FAILED", exception: { - stacktrace: [e.toString().split(/\n/)], + stacktrace: e.toString().split("\n"), }, }, - }; + }); + unexpectedError.name = e.name || "unknown"; await send({ payload: { errors: [unexpectedError], diff --git a/federation-2/router-bridge/src/planner.rs b/federation-2/router-bridge/src/planner.rs index 550a3d8fb..b6c2cafb9 100644 --- a/federation-2/router-bridge/src/planner.rs +++ b/federation-2/router-bridge/src/planner.rs @@ -119,6 +119,15 @@ impl Display for PlanError { pub struct PlanErrorExtensions { /// The error code pub code: String, + /// The stacktrace if we have one + pub exception: Option, +} + +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)] +/// stacktrace in error extensions +pub struct ExtensionsException { + /// The stacktrace generated in JavaScript + pub stacktrace: String, } /// An error that was received during planning within JavaScript. @@ -320,7 +329,6 @@ pub struct PlanErrors { impl std::fmt::Display for PlanErrors { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - dbg!(self); f.write_fmt(format_args!( "query validation errors: {}", self.errors From a1549abb001bd41556558b8e245531632fe03cc7 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 8 Sep 2022 15:32:05 +0200 Subject: [PATCH 5/8] add Clone Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../router-bridge/js-src/plan_worker.ts | 2 + federation-2/router-bridge/package.json | 2 +- federation-2/router-bridge/src/planner.rs | 42 ++++++++++++------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index c3e7db3cd..61cb48521 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -201,6 +201,7 @@ async function run() { }, }); unexpectedError.name = e.name || "unknown"; + await send({ id, payload: { @@ -224,6 +225,7 @@ async function run() { }, }); unexpectedError.name = e.name || "unknown"; + await send({ payload: { errors: [unexpectedError], diff --git a/federation-2/router-bridge/package.json b/federation-2/router-bridge/package.json index 74839a2b2..8fd311a85 100644 --- a/federation-2/router-bridge/package.json +++ b/federation-2/router-bridge/package.json @@ -53,4 +53,4 @@ "node": "16.13.2", "npm": "8.3.1" } -} +} \ No newline at end of file diff --git a/federation-2/router-bridge/src/planner.rs b/federation-2/router-bridge/src/planner.rs index b6c2cafb9..6d6c395ce 100644 --- a/federation-2/router-bridge/src/planner.rs +++ b/federation-2/router-bridge/src/planner.rs @@ -152,7 +152,7 @@ pub struct BridgeSetupResult { pub errors: Option>, } -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] /// The error location pub struct Location { /// The line number @@ -161,7 +161,7 @@ pub struct Location { pub column: u32, } -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] #[serde(untagged)] /// This contains the set of all errors that can be thrown from deno pub enum PlannerError { @@ -198,7 +198,7 @@ impl std::fmt::Display for PlannerError { /// WorkerError represents the non GraphQLErrors the deno worker can throw. /// We try to get as much data out of them. -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct WorkerError { /// The error message pub message: Option, @@ -230,7 +230,7 @@ impl std::fmt::Display for WorkerError { /// We try to get as much data out of them. /// While they mostly represent GraphQLErrors, they sometimes don't. /// See [`WorkerError`] -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] #[serde(rename_all = "camelCase")] pub struct WorkerGraphQLError { /// The error kind @@ -650,7 +650,7 @@ mod tests { assert_eq!( "Syntax Error: Unexpected Name \"this\".", - payload.errors[0].message.as_ref().clone().unwrap() + payload.errors[0].message.as_ref().unwrap() ); assert_eq!( "## GraphQLParseFailure\n", @@ -688,7 +688,7 @@ mod tests { assert_eq!( "Cannot spread fragment \"thatUserFragment1\" within itself via \"thatUserFragment2\".", - payload.errors[0].message.as_ref().clone().unwrap() + payload.errors[0].message.as_ref().unwrap() ); assert_eq!( "## GraphQLValidationFailure\n", @@ -715,7 +715,7 @@ mod tests { assert_eq!( "Unknown operation named \"ThisOperationNameDoesntExist\"", - payload.errors[0].message.as_ref().clone().unwrap() + payload.errors[0].message.as_ref().unwrap() ); assert_eq!( "## GraphQLUnknownOperationName\n", @@ -739,7 +739,7 @@ mod tests { assert_eq!( "Must provide operation name if query contains multiple operations.", - payload.errors[0].message.as_ref().clone().unwrap() + payload.errors[0].message.as_ref().unwrap() ); assert_eq!( "## GraphQLUnknownOperationName\n", @@ -763,7 +763,7 @@ mod tests { assert_eq!( "This anonymous operation must be the only defined operation.", - payload.errors[0].message.as_ref().clone().unwrap() + payload.errors[0].message.as_ref().unwrap() ); assert_eq!( "## GraphQLValidationFailure\n", @@ -787,7 +787,7 @@ mod tests { assert_eq!( "Fragment \"thatUserFragment1\" is never used.", - payload.errors[0].message.as_ref().clone().unwrap() + payload.errors[0].message.as_ref().unwrap() ); assert_eq!( "## GraphQLValidationFailure\n", @@ -885,6 +885,7 @@ mod tests { ), extensions: Some(PlanErrorExtensions { code: "INVALID_GRAPHQL".to_string(), + exception: None, }), }]; @@ -1056,6 +1057,7 @@ GraphQL request:4:1 locations: Default::default(), extensions: Some(PlanErrorExtensions { code: "CheckFailed".to_string(), + exception: None }), original_error: None, causes: vec![ @@ -1063,14 +1065,14 @@ GraphQL request:4:1 message: Some("the `for:` argument is unsupported by version v0.1 of the core spec. Please upgrade to at least @core v0.2 (https://specs.apollo.dev/core/v0.2).".to_string()), name: None, stack: None, - extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string() }), + extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string(), exception: None }), locations: vec![Location { line: 2, column: 1 }, Location { line: 3, column: 1 }, Location { line: 4, column: 1 }] }), Box::new(WorkerError { message: Some("feature https://specs.apollo.dev/something-unsupported/v0.1 is for: SECURITY but is unsupported".to_string()), name: None, stack: None, - extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string() }), + extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string(), exception: None }), locations: vec![Location { line: 4, column: 1 }] }) ], @@ -1114,6 +1116,7 @@ GraphQL request:4:9 locations: Default::default(), extensions: Some(PlanErrorExtensions { code: "CheckFailed".to_string(), + exception: None }), original_error: None, causes: vec![ @@ -1121,7 +1124,7 @@ GraphQL request:4:9 message: Some("feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: EXECUTION but is unsupported".to_string()), name: None, stack: None, - extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string() }), + extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string(), exception: None }), locations: vec![Location { line: 4, column: 9 }] }), ], @@ -1152,12 +1155,14 @@ GraphQL request:4:9 locations: vec![], extensions: Some(PlanErrorExtensions { code: "CheckFailed".to_string(), + exception: None }), original_error: None, causes: vec![Box::new(WorkerError { message: Some("feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: SECURITY but is unsupported".to_string()), extensions: Some(PlanErrorExtensions { code: "UNSUPPORTED_LINKED_FEATURE".to_string(), + exception: None }), name: None, stack: None, @@ -1216,6 +1221,7 @@ mod planning_error { message: Some("something terrible happened".to_string()), extensions: Some(PlanErrorExtensions { code: "E_TEST_CASE".to_string(), + exception: None, }), }; @@ -1290,6 +1296,7 @@ feature https://specs.apollo.dev/something-unsupported/v0.1 is for: SECURITY but locations: Default::default(), extensions: Some(PlanErrorExtensions { code: "CheckFailed".to_string(), + exception: None }), original_error: None, causes: vec![ @@ -1297,14 +1304,14 @@ feature https://specs.apollo.dev/something-unsupported/v0.1 is for: SECURITY but message: Some("the `for:` argument is unsupported by version v0.1 of the core spec. Please upgrade to at least @core v0.2 (https://specs.apollo.dev/core/v0.2).".to_string()), name: None, stack: None, - extensions: Some(PlanErrorExtensions { code: "ForUnsupported".to_string() }), + extensions: Some(PlanErrorExtensions { code: "ForUnsupported".to_string(), exception: None }), locations: vec![Location { line: 2, column: 1 }, Location { line: 3, column: 1 }, Location { line: 4, column: 1 }] }), Box::new(WorkerError { message: Some("feature https://specs.apollo.dev/something-unsupported/v0.1 is for: SECURITY but is unsupported".to_string()), name: None, stack: None, - extensions: Some(PlanErrorExtensions { code: "UnsupportedFeature".to_string() }), + extensions: Some(PlanErrorExtensions { code: "UnsupportedFeature".to_string(), exception: None }), locations: vec![Location { line: 4, column: 1 }] }) ], @@ -1325,6 +1332,7 @@ feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: EXECUTION but locations: Default::default(), extensions: Some(PlanErrorExtensions { code: "CheckFailed".to_string(), + exception: None }), original_error: None, causes: vec![ @@ -1332,7 +1340,7 @@ feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: EXECUTION but message: Some("feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: EXECUTION but is unsupported".to_string()), name: None, stack: None, - extensions: Some(PlanErrorExtensions { code: "UnsupportedFeature".to_string() }), + extensions: Some(PlanErrorExtensions { code: "UnsupportedFeature".to_string(), exception: None }), locations: vec![Location { line: 4, column: 9 }] }), ], @@ -1353,12 +1361,14 @@ feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: SECURITY but i locations: vec![], extensions: Some(PlanErrorExtensions { code: "CheckFailed".to_string(), + exception: None }), original_error: None, causes: vec![Box::new(WorkerError { message: Some("feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: SECURITY but is unsupported".to_string()), extensions: Some(PlanErrorExtensions { code: "UnsupportedFeature".to_string(), + exception: None }), name: None, stack: None, From 05667d8e12d58db0eaa3aaaab0bd1361dc5b0c47 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 8 Sep 2022 16:25:04 +0200 Subject: [PATCH 6/8] cannot use GraphQLError in plan_worker.ts Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../router-bridge/js-src/plan_worker.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index 61cb48521..807a561c9 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -1,6 +1,6 @@ import { GraphQLErrorExt } from "@apollo/core-schema/dist/error"; import { QueryPlannerConfig } from "@apollo/query-planner"; -import { ASTNode, GraphQLError, Source, SourceLocation } from "graphql"; +import { ASTNode, Source, SourceLocation } from "graphql"; import { BridgeQueryPlanner, ExecutionResultWithUsageReporting, @@ -192,15 +192,16 @@ async function run() { } catch (e) { logger.warn(`an error happened in the worker runtime ${e}\n`); - const unexpectedError = new GraphQLError(e.message || "", { + const unexpectedError = { + name: e.name || "unknown", + message: e.message || "", extensions: { code: "QUERY_PLANNING_FAILED", exception: { - stacktrace: e.toString().split("\n"), + stacktrace: e.toString().split(/\n/), }, }, - }); - unexpectedError.name = e.name || "unknown"; + }; await send({ id, @@ -216,15 +217,16 @@ async function run() { } catch (e) { logger.warn(`plan_worker: an unknown error occured ${e}\n`); - const unexpectedError = new GraphQLError(e.message || "", { + const unexpectedError = { + name: e.name || "unknown", + message: e.message || "", extensions: { code: "QUERY_PLANNING_FAILED", exception: { - stacktrace: e.toString().split("\n"), + stacktrace: e.toString().split(/\n/), }, }, - }); - unexpectedError.name = e.name || "unknown"; + }; await send({ payload: { From a96d5501fb3e8c7f3e11368d32d77dcbc6ae355b Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 8 Sep 2022 16:48:57 +0200 Subject: [PATCH 7/8] skip serialize if none Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- federation-2/router-bridge/src/planner.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/federation-2/router-bridge/src/planner.rs b/federation-2/router-bridge/src/planner.rs index 6d6c395ce..b19123d4f 100644 --- a/federation-2/router-bridge/src/planner.rs +++ b/federation-2/router-bridge/src/planner.rs @@ -119,6 +119,7 @@ impl Display for PlanError { pub struct PlanErrorExtensions { /// The error code pub code: String, + #[serde(skip_serializing_if = "Option::is_none")] /// The stacktrace if we have one pub exception: Option, } From 15e788f8d4e0ae91f5e4a63433008e2969d3e951 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Fri, 9 Sep 2022 09:20:18 +0000 Subject: [PATCH 8/8] Try to use the GraphQLError class to produce the error --- .../router-bridge/js-src/plan_worker.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index 807a561c9..951a0d53e 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -1,6 +1,6 @@ import { GraphQLErrorExt } from "@apollo/core-schema/dist/error"; import { QueryPlannerConfig } from "@apollo/query-planner"; -import { ASTNode, Source, SourceLocation } from "graphql"; +import { ASTNode, GraphQLError, Source, SourceLocation } from "graphql"; import { BridgeQueryPlanner, ExecutionResultWithUsageReporting, @@ -192,16 +192,15 @@ async function run() { } catch (e) { logger.warn(`an error happened in the worker runtime ${e}\n`); - const unexpectedError = { - name: e.name || "unknown", - message: e.message || "", + const unexpectedError = new GraphQLError(e.message, { extensions: { code: "QUERY_PLANNING_FAILED", exception: { stacktrace: e.toString().split(/\n/), }, }, - }; + }); + unexpectedError.name = e.name || "unknown"; await send({ id, @@ -215,18 +214,17 @@ async function run() { }); } } catch (e) { - logger.warn(`plan_worker: an unknown error occured ${e}\n`); + logger.warn(`plan_worker: an unknown error occurred ${e}\n`); - const unexpectedError = { - name: e.name || "unknown", - message: e.message || "", + const unexpectedError = new GraphQLError(e.message, { extensions: { code: "QUERY_PLANNING_FAILED", exception: { stacktrace: e.toString().split(/\n/), }, }, - }; + }); + unexpectedError.name = e.name || "unknown"; await send({ payload: {