-
-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stitching with formatError is very limited... #480
Comments
I ran into this today. Needs a fix. Very hard to debug. I had a subscription field returning a list which was returning this in graphql: The issue was one of the fields in the list items missing. There is no way to see the error without turning off stitching. This should be labelled as bug, not enhancement. |
i agree with @vjpr on this one. this issue should be marked as a bug and not an enhancement (@freiksenet ) im surprised there aren't more people complaining about this issue. is there a workaround that we're not aware of? how are other people debugging their local development with schema stitching when the stack traces are useless? |
This is a bug. It's impacting us. |
This is also impacting us. We were unable to pass custom error codes because the merged schema swallows the thrown errors. |
+1 |
We've also run into this, and had to turn off stitching to debug. |
The issue can be closed given #637 was merged an released. I had to add some ugly error drill-down logic, at least, it's working. |
I am still having issues viewing any extra properties associated with an error inside formatError:
Inside
output:
This only occurs when using Update: I managed to finally find my error object here:
|
I was also just bit by this. It took ~2 days to track down the issue. Locally generated errors should maintain their types and custom fields when passed into |
In my case, I have micro-services connected to an api gateway that makes the stitching. In User micro-service const {
SchemaDirectiveVisitor,
AuthenticationError,
ForbiddenError
} = require('apollo-server-express');
// ...
if (!context.token) {
throw new AuthenticationError('User not authenticated');
}
// ... In Api Gateway function searchOriginalError (error) {
if (error.originalError) {
return searchOriginalError(error.originalError);
}
if (error.errors) {
return error.errors.map(searchOriginalError)[0];
}
return error;
}
module.exports = { searchOriginalError }; In Api Gateway const server = new ApolloServer({
schema,
context: ({ req }) => {
// ...
},
formatError: (error) => {
const originalError = searchOriginalError(error);
logger.error(originalError);
return originalError;
}
}); I hope, this helps. |
For us, using formatError and simply returning the error with NO changes impacts behavior. That's silly right? E.g.
This ^ behaves differently than no formatError We are using schema stitching... it seems like data is stripped or removed from the errors only if we have a formatError function defined. |
My team has been playing with stitching and attempted many of the suggestions above (and related issues). We appear to be hitting the issue of exceptions being swallowed versus having to unwrap. Has anyone actually figured out how to override the internals of graphql-tools to monkeypatch the code to work around this? If we add debug console output for the unwrap,
we see the following..
The result to the client request becomes
|
Unfortunately @OlivierCuyp's solution didn't work in our case either. The upstream service errors weren't being exposed to This flow, (flow 1) However the subsequent (flow 2) Narrowed it down to this line in checkResultAndHandleErrors. When this error is thrown, it's received by the Workaround was to add an error handler to the
Then flow 2 received an error with the right context that was then conveyed to |
@smber1 thanks for sharing it, it works for me |
Hmmm... I've tried almost all of the approaches outlined in this thread and I just can't get any of them to work. In my system we have a main API gateway which links to other schemas. The flow we're following is pretty close to the docs let remoteLink = new HttpLink({ uri : link.path, fetch });
let remoteSchema = await introspectSchema(remoteLink);
validateSchema({ schema : remoteSchema, prefix : link.name });
let remoteExecutableSchema = makeRemoteExecutableSchema({
schema : remoteSchema,
link : remoteLink
});
// later
var schema = mergeSchemas({ schemas }); I am able to access the errors with the proper stack trace if I utilize the Right now apollo has settings so that it will display stacktraces based on |
Not saying this is the best solution but we have used an approach where we create a custom formatError for each apollo server instance unpacks the error and throws it again as an ApolloError const stringify = require('json-stringify-safe');
const defaultFormatErrorHandler = (error) => {
const err = JSON.parse(stringify(error));
console.log(err);
delete err.extensions.exception.config;
delete err.extensions.exception.request;
delete err.extensions.exception.response;
return new ApolloError('ERROR', err.extensions.code, { exception: { ...err } });
};
new Apollo({
formatError: defaultFormatErrorHandler,
...
}) It is very hacky and the actual error gets buried one level deeper than necessary but it does the job of preserving the error code and the message in the first error layer which can then be inspected by the client. Example error from our implementation {
"errors": [
{
"locations": [],
"path": [
"updateSektion"
],
"extensions": {
"code": "SECTOR9_NOT_LOGGED_IN_ERROR",
"exception": {
"exception": {
"message": "Principal is not logged in",
"locations": [
{
"line": 2,
"column": 3
}
],
"path": [
"updateSektion"
],
"extensions": {
"code": "SECTOR9_NOT_LOGGED_IN_ERROR",
"exception": {
"invalidJWT": true,
"stacktrace": [
"Error: Principal is not logged in",
" at Principal.requireLogIn (/home/node/app/node_modules/sector9/build/Principal.js:73:19)",
" at exports.updateSektionMutation (/home/node/app/build/graphql/schemas/sektion/mutation.js:5:10)",
" at field.resolve (/home/node/app/node_modules/apollo-server-core/node_modules/graphql-extensions/dist/index.js:128:26)",
" at resolveFieldValueOrError (/home/node/app/node_modules/graphql/execution/execute.js:479:18)",
" at resolveField (/home/node/app/node_modules/graphql/execution/execute.js:446:16)",
" at /home/node/app/node_modules/graphql/execution/execute.js:262:18",
" at /home/node/app/node_modules/graphql/jsutils/promiseReduce.js:32:10",
" at Array.reduce (<anonymous>)",
" at promiseReduce (/home/node/app/node_modules/graphql/jsutils/promiseReduce.js:29:17)",
" at executeFieldsSerially (/home/node/app/node_modules/graphql/execution/execute.js:259:37)"
]
}
}
},
"stacktrace": [
"GraphQLError",
" at Object.locatedError (/home/node/app/node_modules/graphql/error/locatedError.js:31:10)",
" at Object.checkResultAndHandleErrors (/home/node/app/node_modules/graphql-tools/dist/stitching/errors.js:99:23)",
" at /home/node/app/node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.js:159:52",
" at step (/home/node/app/node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.js:31:23)",
" at Object.next (/home/node/app/node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.js:12:53)",
" at fulfilled (/home/node/app/node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.js:3:58)",
" at process._tickCallback (internal/process/next_tick.js:68:7)"
]
}
}
}
],
"data": {
"updateSektion": null
}
} |
Those of you who just want to be able to use two endpoints can use Directional Composition. That is if you don't want to deal with the issues with stitching. But I think directional composition only works with two endpoints, not more. apollographql/react-apollo#1863 (comment) |
Was this issue fixed? Thanks
|
This should be fixed in graphql-tools-fork. Please let me know if not working for you. |
Using |
If you could share a reproduction, I could take a look? |
Thanks for help, the problem is in the |
I use this:
import { onError } from 'apollo-link-error';
...
const ErrorLink = onError(({ graphQLErrors, response }) => {
if (graphQLErrors && graphQLErrors.length === 1) {
graphQLErrors.length = 2;
}
});
import { formatApolloErrors } from 'apollo-server-errors';
...
const formatError = (error) => {
let errors = [];
if(!error.originalError || !error.originalError.errors || error.originalError.errors.length <= 0) {
return error;
}
error.originalError.errors.forEach((currentError) => {
if(!currentError) return;
let currentOriginal = currentError.originalError;
const compiledErrors = formatApolloErrors(currentOriginal.errors || []);
if(compiledErrors === null) {
return;
}
errors.push(...compiledErrors.filter(err => !!err));
});
return (errors.length > 1) ? errors : errors[0];
}; For result: I throw Forbidden exception in the microservice and get response in the gateway service:
And if exception thrown in the getaway service:
|
Has anyone tried this with apollo federation? |
Should be fixed by #1307. |
Not sure if it's related, but I encountered a similar issue where an error on a field in an underlying schema was being ignored by the stitched schema gateway. The gateway would try to return I am using It looks like this is caused by the code in My solution for now has been to use a transformer that throws on all errors, like this: import { CombinedError, stitchSchemas } from 'graphql-tools';
const throwOnAllResultErrors = {
transformResult(result: any) {
// always throw if an error is returned by the underlying schema
if (result.errors != null && result.errors.length > 0) {
throw new CombinedError(result.errors);
}
return result;
}
};
stitchSchemas({
subschemas: [
{
schema,
transforms: [throwOnAllResultErrors],
},
{
remoteSchema,
transforms: [throwOnAllResultErrors],
},
]
}); |
Thanks for sharing your workaround. It would be extremely helpful if you could also open a new issue with a minimal reproduction, including:
Thanks! |
Your workaround would seem to prevent partial GraphQL response, right? |
If you want to format your errors when using stitching, you can't tell the error type from the error.
The issue seems to be located here:
https://github.com/apollographql/graphql-tools/blob/master/src/stitching/errors.ts#L80
So packages like
apollo-errors
don't work because you can't get the type from it.One solution I think I see that's somewhat possible is to check if it's a schema created for a remote server and if so, run
checkResultAndHandleErrors
, otherwise, just throw the error as is sincecheckResultAndHandleErrors
looks to be made for remote responses, and if it's local, the errors should just be thrown normallyhttps://github.com/apollographql/graphql-tools/blob/9ac0aeb25cd6c99d2e1ce6c14809d64f85b87eaf/src/stitching/mergeSchemas.ts#L322-L330
Thoughts?
The text was updated successfully, but these errors were encountered: