Best-effort sideloading of resources

[CGA1123]

Originally part of #333, splitting into a new issue to allow for more focused conversation.

From https://github.com/graphiti-api/graphiti/issues/333#issue-828507665

(...) should be any scope for "best-effort" sideloading to be enabled, where if a remote resource cannot be fetched due to returning some subset of HTTP status codes the whole request is not necessarily failed. And clients are expected/allowed to retry fetching that failed resource. This would allow for more graceful degradation of service should an upstream API be unavailable or excessively latent for whatever reasons.

I'm not sure whether that suggestion is conformant to the JSONAPI spec, the section on "includes" leaves this a little bit ambiguous as far as I can tell. It might be best to avoid this additional complexity for now. 😄

From https://github.com/graphiti-api/graphiti/issues/333#issuecomment-797037051

(...) You could make the same case for a local API - if we can fetch the Employees, but Positions fails, should the whole request fail? Currently we say yes. GraphQL would often say no, I think. Ideally we would make this configurable when defining the sideload (e.g. has_many :positions, allow_errors: true) with the current behavior being the default but JSON:API says:

The members data and errors MUST NOT coexist in the same document.

So we'd have to return a successful response without telling the user anything failed. TBQH I think it is OK to cheat here 😈 (...)

[CGA1123]

So we'd have to return a successful response without telling the user anything failed. TBQH I think it is OK to cheat here 😈

This might cause quite a few JSONAPI clients to break, including Graphiti itself, which raises an Errors::Remote exception if the response payload has a errors key!

I think "silently" omitting the sideloads might be a better choice, some clients already have this functionality where they check the included resources before attempting to load based on the relationships links.

[richmolj]

Ha, really good point!

How would the client distinguish between a has_many being an empty array, versus having an error loading the relationship? I could see putting an error on the relationship meta?

Where my head's at: I tend to lean towards supporting both ways with a config option that defaults to rendering errors alongside data (and updating Errors::Remote for this). This matches how GraphQL does it, which not only brings us in-line with a broader community but helps when adding GraphQL support to Graphiti (which I am currently doing!). The other small advantage here is clients can still say "if there's ANY error, fail the whole thing" very easily instead of walking the payload tree.

I could support the silent option as less/more straightforward work (where we could add the option later and default to silent)...but I'm not sure that's the case if we have to touch the relationship meta.

[CGA1123]

Would a potentially simpler solution be not to touch the relationship meta but use the top-level document meta instead?

This should make it relatively simpler to aggregate all sideloading failures at a request context level and add to the finalised document without having to add too much logic to the current relationship building/rendering code?

An approach like this would also lend itself to a easily implementable toggle between :graphql and :jsonapi formats, where depending on the format option errors would be populated under the errors or meta keys in the top-level JSON object?

[richmolj]

Yeah, that's an elegant solution. Conceptually the same as top-level errors without violating the spec. I'd be down 👍

[CGA1123]

Just copying this over from https://github.com/graphiti-api/graphiti/issues/333#issuecomment-801977854 around the potential for allowing configuration of best-effort behaviour at the resource or request level:

It might be interesting to consider whether the client should be allowed to request the level of fidelity of the includes that it requires/wants? Different clients may decide to be more or less tolerant to sideloading errors and decide to show partial information or try to fetch it themselves via links.

Also smart thought. I wonder if we should do this similar to how we do link rendering, which can be configured either by url param OR a resource config option (probably sideload config option in this case).