allow fetch to fail cleanly

[tpolecat]

This changes SqlModule.fetch to yield a Result, which allows fetch to fail with cleanly (we are overriding fetch to fail on excessively large rowsets and prefer not to use an exception for this).

This also pulls out rowDecoder in SkunkMapping to allow access by overridden fetch implementations.

[milessabin]

Instead of exposing internals, how would you feel about adding the FetchLimit functionality here directly?

[tpolecat]

Yes, I think it can probably be factored into something prettier but I'm not 100% sure what I need yet, beyond the ability to make fetch fail. So I'd like to make this minimal change for now and perhaps revisit.

[milessabin]

How are you thinking about representing failures within Result? Result captures a success, or a list of GraphQL errors which should be reported to the client, or an internal error which is a Throwable. If it's the Throwable then I'm not sure I understand why it can't be returned in the F? Are you really intending for these to go back to the client, or is the plan to intercept them somewhere along the line and recover somehow?

[milessabin]

I'm fine with rowDecoder being exposed, BTW.

[tpolecat]

Yes, the error will go back to the client with an explanation that their query was too complex. The alternative is to raise a special exception and catch it up top and turn it into a result, which is less appealing.

[milessabin]

I'm still a little uncomfortable with this ... everything GraphQL related has been eliminated from the arguments and results of fetch so it seems odd that something GraphQL related should sneak out in the error channel an extra error channel added just for that purpose. It feels like a layering violation.

I'm more than happy to merge the part that exposes the rowDecoder, and I'll merge the other part if you're adamant, but it really feels to me that this sort of error should be returned in the F and turned into a GraphQL error somewhere where you actually have some GraphQL context.