Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

ssr: on apollo error, error not passed to component #3918

Open
maapteh opened this issue Apr 8, 2020 · 6 comments
Open

ssr: on apollo error, error not passed to component #3918

maapteh opened this issue Apr 8, 2020 · 6 comments

Comments

@maapteh
Copy link
Contributor

maapteh commented Apr 8, 2020

Intended outcome:
When having an Apollo Error thrown via GraphQL, the React ssr treewalker should just take the Error and proceed and pass error to the component.

Actual outcome:
When we have one query with an Apollo Error, which get sent in batch to server, then the treewalker just throws the Error! and breaks your application from rendering at all. When we catch it with the errorPolicy 'all' then it will not throw the error while treewalking. But this error never reaches the component, while documentation stated it does (saves both data and errors).

For client-side everything works as it should. The errors are passed to error object, data is null.

So when adjusting errorPolicy to 'all' the error is not thrown during tree walking BUT the error also doesn't reach the component. And this is not according to docs: "all: Using the all policy is the best way to notify your users of potential issues while still showing as much data as possible from your server. It saves both data and errors into the Apollo Cache so your UI can use them."

Im really lost now, and the whole thing that i let my "component react on the error" thought is gone as soon as i move client-side parts into server-side parts. I really think more apps/users will expect the same behavior and will be really surprised, or worse not knowing they will never show the error to their visitor?

How to reproduce the issue:
Have one query directly sent an Error on ssr, see my reproduction running at https://codesandbox.io/s/github/maapteh/sandbox-react-apollo-issues-3918

Version

"@apollo/react-common": "3.1.4",
"@apollo/react-hooks": "3.1.5",
"@apollo/react-ssr": "3.1.5",
"@apollo/react-testing": "3.1.4",

"apollo-cache-inmemory": "1.6.5",
"apollo-client": "2.6.8",
"apollo-link-batch-http": "1.2.13",
"apollo-link-error": "1.1.12",
"apollo-link-http": "1.5.16",
"graphql": "14.6.0",
"graphql-tag": "2.10.3",

Issue was related to #1403, but that one got closed and seems to not getting any response anymore so opening new one.

Edited: sandbox created at https://codesandbox.io/s/github/maapteh/sandbox-react-apollo-issues-3918 this makes it hopefully more clear. Sorry for closing and reopening this issue, had weird behaviour in my production application when switching to yarn.

Happy to help you further @hwillson

@maapteh maapteh closed this as completed Apr 8, 2020
@maapteh maapteh reopened this Apr 8, 2020
@maapteh
Copy link
Contributor Author

maapteh commented Apr 16, 2020

If you set the default options it will work partly. It will not throw the error. I dont know who in hell ever wants his app tree walker to literally throw an error but hey :) So this will fix your issue concerning the error being thrown while treewalking:

    const client = new ApolloClient({
        cache,
        resolvers: {},
        ...SNIP
        queryDeduplication: true,
        defaultOptions: {
            mutate: {
                errorPolicy: 'all',
            },
            query: {
                errorPolicy: 'all',
            },
        },
    });

BUT the error will never reach its component, and we need it :(

@maapteh maapteh closed this as completed Apr 16, 2020
@maapteh maapteh reopened this Apr 16, 2020
@maapteh maapteh closed this as completed Apr 16, 2020
@maapteh maapteh reopened this Apr 16, 2020
@michaelchiche
Copy link

@maapteh you should create a codesandbox repro

@maapteh
Copy link
Contributor Author

maapteh commented Apr 16, 2020

Yes maybe i will, point is then that i have to spin up the api and web and hugh already has my sample app which is more easier to share :)

Sorry for all this opening and re opening emails people :(
Also when i pin i expect it to use a certain version which is why i open and re-opened so many times because of under the hood it was using weird versions:

├── @apollo/react-common@3.1.3 
├─┬ @apollo/react-hooks@3.1.3
│ └── @apollo/react-common@3.1.4 
├─┬ @apollo/react-ssr@3.1.3
│ ├── @apollo/react-common@3.1.4 
│ └─┬ @apollo/react-hooks@3.1.5
│   └── @apollo/react-common@3.1.4  deduped
└─┬ @apollo/react-testing@3.1.3
  └── @apollo/react-common@3.1.4 

When i moved to 3.1.5 (and 4 in some cases) the client side refetch was not done anymore.

So my api throws error:

[api] {"message":"foo bar","locations":[{"line":2,"column":3}],"path":["member"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: foo bar","    at Object.exports.memberDataLoader (/SNIP/dataloaders/member.dataloader.ts:18:11)","    at process._tickCallback (internal/process/next_tick.js:68:7)"]}}}
[api] DL: product [ 61130, 3202 ]

But on web part its not being picked up, error is blank and not retrieved again.

@maapteh
Copy link
Contributor Author

maapteh commented Apr 17, 2020

Sandbox created, will also update the issue accordingly:

https://codesandbox.io/s/github/maapteh/sandbox-react-apollo-issues-3918

So this app shows how to prevent the error being thrown on treewalker but also that there is no error for the component to handle :( When you remove errorPolicy then treewalker throws (which we catch there) but then the call is also done client-side and then fills the error. But that puts load on my server twice. I just want to throw the error once :)

Readme with in depth explanation can also be found here: https://github.com/maapteh/sandbox-react-apollo-issues-3918/tree/master/

@maapteh maapteh changed the title ssr: on apollo error the treewalker has no catch and breaks whole app ssr: on apollo error, error not passed to component Apr 17, 2020
@maapteh
Copy link
Contributor Author

maapteh commented Apr 18, 2020

Im now finding more and more issues related to this bug, like apollographql/apollo-client#4644

When i started looking in the react package i only found this closed one. Hopefully the sandbox helps in identifying the rootcause and close these issues and make lots of users happy :)

begelundmuller added a commit to beneath-hq/beneath that referenced this issue Apr 21, 2020
@maapteh
Copy link
Contributor Author

maapteh commented Apr 30, 2020

@hwillson or @benjamn or ??? if i would know how to solve it, i would have made a PR. But me and my team is super lost in this, and it also seems to be a bug for a long time (went to latest alpha and back to 2). Is it possible to give some hints, or even better fix it together?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants