Skip to content
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

Schema stitching swallows original error stack traces from the children #743

Closed
1 of 5 tasks
stringbeans opened this issue Apr 17, 2018 · 27 comments
Closed
1 of 5 tasks
Assignees
Labels
has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository v5

Comments

@stringbeans
Copy link
Contributor

This is related to the issue here: #480 and the (now accepted and closed) PR here: #637

The above PR unfortunately only fixes the case when there is an error thrown from a PARENT object in the response. so for example, if the query is something like:

query {
  student(id: 5) {
    studentId
  }
}

HOWEVER, if there is an error thrown within a child field/object's resolver then graphql's schema stitching still swallows the error. example:

query {
  student(id: 5) {
    studentId,
    favouriteColor //if sub-resolver throws error here, stack trace will be swallowed
  }
}
  • has-reproduction
  • feature
  • docs
  • blocking
  • good first issue
@ghost ghost added the has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository label Apr 17, 2018
@mfix22
Copy link
Contributor

mfix22 commented May 8, 2018

@stringbeans have you tried retrieving the original error from like:

err => err.originalError.errors

?

@stringbeans
Copy link
Contributor Author

@mfix22 yes i have... i actually was the one that fixed the issue originally but discovered that it didnt fix the case for sub-resolvers as described in the original description :(

@codingedgar
Copy link

I do have this same error currently, is there any workaround to mitigate this until fixed?

@bazaglia
Copy link

Waiting for a workaround also... This is pretty pretty annoying.

@codingedgar
Copy link

Any workaround found for this issue?

@stubailo
Copy link
Contributor

Gonna try to fix this today! Starting to try to unpack the error handling stuff at the moment.

@stubailo stubailo self-assigned this Aug 10, 2018
@bazaglia
Copy link

@stubailo Hey Sashko!

Just a background info: in my case I have a GraphQL microservice-based architecture and a gateway that merge all my schemas in front of the microservices which is my GraphQL exposed API. But in this merge step, the schema stiching swallows the original error so it's almost impossible to debug an error (no stack trace).

@low-ghost
Copy link
Contributor

This is where the error loses additional context, at least in my particular case: https://github.com/apollographql/graphql-tools/blob/master/src/stitching/errors.ts#L98-L108. I don't know enough about apollo-link-http and http-link-dataloader, but I think that comment only applies to the condition for is singular in length and hasResult, which apollo-error instances at least do not pass.

So, there's the conceptual issue of trying to throw a singular error for multiple instances as a CombinedError. That might be the intended approach, but it definitely breaks the consumer's formatError, unless I'm missing the part where this.errors is parsed out into multiple errors again.

Can't the error instances just be returned as the errors key and graphql will propagate them up correctly? Isn't that essentially the point of annotateWithChildrenErrors just above this? But, take all this with the grain of salt that I'm just starting to look into it

@AdelBachene
Copy link

I have the same Issue and I overcame it by using originalError object

import {isInstance as isApolloErrorInstance, formatError as formatApolloError} from 'apollo-errors';

                 formatError: (error) => {
                        let errors: any[] = [];
                        error.originalError.errors.forEach((currentError) => {
                            let currentOriginal = currentError.originalError;
                            if (isApolloErrorInstance(currentOriginal)) {
                                Logger.Instance().gatewayError(currentOriginal);
                            }
                            errors.push(formatApolloError(currentOriginal, false));
                        });
                        return (errors.length > 1) ? errors : errors[0];
                    },

@OlivierCuyp
Copy link

@AdelBachene I had the same issue but my orignal error was nested deeper.
I posted my solution here:

#480 (comment)

@dwelch2344
Copy link

Figured I'd chime in here. I'm running into the scenario where we're losing all errors and getting a message of [Object object] when resolving through stitching, and I think it's related.

We've got the following architecture:

Proxy GraphQL  ---[remote stitch]---> Main GraphQL ----- [local stitch]---> [Schema1, Schema2]

So if our clients hit the Main GraphQL directly, we see everything as expected. If they then hit the Proxy GraphQL (needed for some augmentation / stitching other sources) we only get back

{ "message": "[Object object]", ...}

What's interesting is that if we provide formatError in the Main GraphQL's ApolloServer and we delete the the extensions property on the default payload there, everything works as expected.

I need to read more about how the format of those errors should be, but figured it was interesting enough to provide

@ghost
Copy link

ghost commented Nov 25, 2018

@dwelch2344 I've had the same experience. The cause is outlined in my issue dotansimha/graphql-binding#173

@alfaproject
Copy link

alfaproject commented Nov 28, 2018

Just spent a good 4h trying to debug my custom link thinking that I was doing something wrong and not understanding why my error message is [object Object] from the downstream stitched GraphQL server...

Is there any good workaround? I've ended up checking if the result has errors and if it does, then I do observer.error(new Error(result.errors[0].message)); but this is filthy. ):

P.S.: My architecture is the exact same as @bazaglia and @dwelch2344 except I use a different transport/link.

@inakianduaga
Copy link

I can't get any of the solutions to work, seems bug is pretty important since it affects core schema stitching functionality (in my case basically stitching just a single schema is enough to swallow the exceptions within that one remote schema)

@devtobo
Copy link

devtobo commented Nov 30, 2018

@OlivierCuyp Nice, good use of the recursive function
Thanks for sharing

@jimmycallin
Copy link

Seeing this as well, and it severely affects the development experience when using graphql with schema stitching.

@hwillson
Copy link
Contributor

hwillson commented Mar 7, 2019

Here's a runnable reproduction that demonstrates this issue:
https://github.com/hwillson/graphql-tools-stitching-errors

I'm working on a fix now, and will have it ready shortly.

@owenallenaz
Copy link
Contributor

I'm having issues similar to this, but it seems to depend on how deeply nested the error occurs within my schema stitched resolver. In example my graphql query could be like so { auth { accounts { count } } } if the auth resolver throws an error, the above mechanics work find to get around the object object issue. But if accounts throws the error, my stack trace is lost, and I cannot find a way to preserve the original error code. It's behaving as if it's creating a new error from scratch. When I go directly to the stitched graph endpoint, both the nested error and the root level error appear to be getting the same data when I console log them in the formatError mechanic so I think it's the stitcher server that is mishandling the error object.

@szaza
Copy link

szaza commented Mar 26, 2019

Hi, I had the same issue. I managed to resolve it by using the workaround what @smber1 and @Pajk recommended in the threads:

Currently my introspection.ts looks like this:
https://gist.github.com/szaza/12b079129283197036bcf96124b0af10;

I've also had to format the error messages in the child nodes and the root node like this:
https://gist.github.com/szaza/2966f8c22b2205330e9b9d3d0835a541

yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Jun 12, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Jun 18, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
@alfaproject
Copy link

No hope to get this sorted? ):

@maxmoeschinger
Copy link

Seems like this is only happening when there are multiple errors thrown in the resolvers. What is the ETA on this? Seems like multiple persons have tried to fix this but nothing has happened?

yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Sep 22, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Sep 22, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Sep 22, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Oct 3, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
@hnrklssn
Copy link

Solved this by switching to using graphql-tools-fork in our gateway until @yaacovCR's PR has been merged. Didn't need to change anything other than package.json and all the imports of graphql-tools, and worked like a charm. Thank's mate!

yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Oct 25, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Oct 25, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Nov 4, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Dec 31, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Dec 31, 2019
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
@intellix
Copy link

intellix commented Jan 6, 2020

I guess there's too much required work to provide a patch-package and the only actual solution as of now is to switch to graphql-tools-fork?

This branch is 190 commits ahead, 51 commits behind apollographql:master.

Really wanted to use extensions.code in our frontend to hide Internal Server Errors as they're a terrible UX

@yaacovCR
Copy link
Collaborator

yaacovCR commented Jan 6, 2020

#1206 is up to date on a separate forkPR branch I am maintaining.

As it has not been merged for a while now, that likely does not change your main point, which is that the solution for right now seems to be to use the fork.

:(

Hopefully, the PR will be merged soon?

yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Jan 8, 2020
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Jan 21, 2020
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Feb 27, 2020
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Mar 26, 2020
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
yaacovCR pushed a commit to yaacovCR/graphql-tools-fork that referenced this issue Mar 26, 2020
Use new relocatedError function to update the original GraphQLErrors
with the new path. Addresses ardatan#743, ardatan#1037, ardatan#1046,
apollographql/apollo-server#1582.
@kamilkisiela
Copy link
Collaborator

We recently released an alpha version of GraphQL Tools (#1308) that should fix the issue.

Please update graphql-tools to next or run:

npx match-version graphql-tools next

Let us know if it solves the problem, we're counting for your feedback! :)

@yaacovCR yaacovCR mentioned this issue Mar 27, 2020
22 tasks
@yaacovCR
Copy link
Collaborator

Rolled into #1306

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository v5
Projects
None yet
Development

No branches or pull requests