Fork me on GitHub
#graphql
<
2021-02-04
>
Lennart Buit14:02:24

Hey. If a lacinia resolver throws an exception, this exception is passed back in a resolve-promise as a value (here: [1]). Now, if a resolver returns a resolve-promise itself, and crashes in trying to do something asynchronously, we can’t seem to pass the exception we got to lacinia-pedestal: we can’t throw it because the resolver already completed, its lacinia waiting for us to deliver on our result-promise We can’t deliver the exception as value to our result-promise as it then appears to be read as a ‘normal’ successful value (here: [2]). What is the conventional wisdom here? [1]: https://github.com/walmartlabs/lacinia/blob/83ee169e4aa436bba56b8b725a03aae0e6bffb26/src/com/walmartlabs/lacinia/executor.clj#L465-L466 [2]: https://github.com/walmartlabs/lacinia/blob/83ee169e4aa436bba56b8b725a03aae0e6bffb26/src/com/walmartlabs/lacinia/executor.clj#L450-L464

1
hlship16:02:33

You can use a try block, and, convert the exception to an error map, then wrap your result (even nil) via resolve/with-error. The 3-arity deliver! method is just a convenience around with-error (well, it predates with-error, but still).

thumbnail17:02:39

Would there be a way to get the ex through the pedestal stack? Handeling exceptions is already setup there (monitoring, reporting, default internal-server-error-response etc).

hlship17:02:32

There really isn't, resolver functions aren't supposed to fail, they're supposed to deliver an error map. We catch exceptions when calling the field resolver initially (mostly as a convenience during development).

hlship17:02:12

One approach that might work is to add an interceptor to the chain that puts an Atom into the context; on leave the value in the Atom is deref'ed and, if non-nil, re-thrown.

hlship17:02:39

But this is an interesting use case and I have to wonder if there's a way to restructure things to make it easier to propogate a thrown exception out from even an async callback but maintain the useful things that Lacinia does (identify field, arguments, etc.) when an exception is thrown by synchronous field resolver.

👀 1
thumbnail17:02:03

The atom suggestion is pretty good. Could even watch the atom and halt the resolving upon an uncaught exception. All 'expected' exceptions are handled in the resolver and are returned using with-errors or a similar construct. For any exception we didn't expect, we use an interceptor as a last-line-of-defence. It which logs the error, pushes it to rollbar (/sentry) and returns a generic error message to the frontend. In a perfect world that interceptor wouldn't be hit, but you know... After seeing the link @UDF11HLKC shared, I assumed lacinia to rethrow if a throwable was delivered to a ResolverResult. But I guess that's on me 😅

hlship18:02:52

Nailing down the right approach to exceptions is always a challenge, and that goes double for any kind of asynchronous processing. We have similar issues in use of core.async. I like how in Elixir, it's always about a receive and that always has a timeout.

👀 1
hlship18:02:37

Sorry, segued there from exceptions to dealing with async in general (there's an overlap, where exceptions may impact delivery of async results).

Lennart Buit20:02:27

Yeah agree, in lacinia-pedestal (or pedestal for that matter) we (or I) had the same issue in dealing with an exception where there were also differences in how to handle sync/async exceptions. Iirc, you can ‘just’ throw in a sync interceptor, but you need to return an ex in a specific way in the context in an async one. Certainly not an easy issue

Lennart Buit20:02:41

Maybe that’s something to take inspiration from, pedestals context contains an error key for asynchronous delivery of exceptions. Definitely didn’t think this through, but sounds like something that could work (e.g. resolve/with-exception)

hlship23:02:31

That's an idea.