-
Notifications
You must be signed in to change notification settings - Fork 29
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
Handing GraphQL server errors #15
Comments
Ah great question, we were facing the same thing. Our app has some public pages, some pages that require authentication, and some pages that require authorization. We couldn't find a general "Not authorized" solution we were happy with because:
What we ended up doing was letting Next.js take care of showing an error page when the GraphQL query failed to load data, and trying to not show users links to things they can't access. |
Thanks for your answer @rrdelaney! That's interesting. What about redirects that relate to domain logic, for example redirecting to an onboarding page if the user has not setup certain data? do you do it client side in this case? Cheers 🍺 |
Ah yea in that case you can return an object from
Great question by the way, we should definitely add this to the docs. |
@rrdelaney just to be sure I understand correctly, I think the
Thanks for your patience 🙇 |
Hm yea that one isn't possible at the moment. We don't expose a generic hook between "execute the query" and "render data" because that might be happening on either the client-side or the server-side. I think the main use-cases there would be showing 404 pages and redirects. What you could do is execute the redirect in the network layer on the client and in |
To be certain, For example, right now, we're redirected to a
I'm unsure how this would help? If this cannot happen between "execute query" and "render data", then what options are we left with? Also, we're unable to use For additional context, here's a solution I was considering (using the repo example as a reference)
However, even if I get as far as to pass back errors silently, I still have nowhere to deal with this error, since Any thoughts on this? |
In general, your GraphQL API should probably not be throwing errors for the queries needed to render a page. You can defer the parts likely to throw an error with a Re |
Totally hear you on this. Maybe I can give you some examples and you can point me in the right direction in terms of best practice (knowing where this tool will go, has been, etc)? Appreciate your fast response and insight on this btw!
(less important: same as above, except for possible 500. although, to your point, this shouldn't happen)
Ah, got it! Thanks, wasn't sure if I was missing something 🙏🏻 |
Ah yea handling 404's like that can be tricky, I've had trouble doing it in our own application. We've been using |
I wonder, is there a way to get the original requested route, params and query from 404? That may help a bit actually o_0 Just spitballing |
I've created a PR which allows to access query data and do redirects and set status codes based on that during SSR: #74 I think it'd be possible to modify so that you can return specific errors etc as well to be passed as prop, but I don't have a need for it at the moment so didn't really think about it. |
First of all thanks for this great package, super helpful ❤️
I'm trying to implement a redirect to login when my GraphQL server responds with a 401. One of the examples in this repo handles this by checking for valid auth before the GraphQL request which works but is not ideal given I'd need to make an auth check on every request.
It sounds like the best way to redirect on 401 would be to
throw
in the relay network/env and somehow catch it somewhere ingetServerSideProps
, so that we're able to return a nextjs redirect.Unfortunately I don't think it's possible to handle this logic in userland through the
getServerSideProps
option this package provides, because that happens beforeloadQuery
is called. https://github.com/RevereCRE/relay-nextjs/blob/main/src/wired/component.tsx#L81-L116Is there a better way to do this? Happy to contribute something if needed 🙇
The text was updated successfully, but these errors were encountered: