feature(apollo-gateway): Add exception option to throw on service connection failure

[jmatsushita]

Adds an option per service in the serviceList object array, to allow a failure to connect to throw an exception at load time, thus allowing the caller to add retry logic. This change is backwards compatible so that the default will be a console.warn as it was.

[apollo-cla]

@jmatsushita: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[trevor-scheer]

Thanks for the contribution, @jmatsushita! I appreciate the docs addition and backward-compatible consideration.

I do think this is interesting, and it got me thinking about some (not necessarily better) alternatives. I'd like to hear your thoughts! Any additional explanation about your use case for this would be great (i.e. why the alternatives below may be any better or worse).

1. Should this be configurable per service? I imagine a strong majority that uses this behavior would want it to be consistent for all services. This would weaken the flexibility but simplify the API and usability.

2. Should this be a boolean? I'm toying with the idea of a callback instead, something like (for lack of a better name) onConnectionFailure, which would provide more flexibility to the implementor for error handling.

[jmatsushita]

Hi @trevor-scheer! Thanks for the kind words and thoughtful remarks. You're definitely right that a callback is more flexible than a boolean triggering an exception.

I would argue that configurability per service is useful though, since there could be very different data infrastructure on the back of each federated services with different failure modes and different impact on the gateway itself (i.e. how do failures compose). For instance do we return an error at the gateway for any downstream connectivity failure? Do we return an empty sub-graph? A default sub-graph? In other words, the callback should probably be able to modify the query result.

A further thought. This will trigger on a federated service connection failure on .load() do you have any thoughts regarding behavior when an already loaded and running gateway loses connectivity?

[abernix]

Thanks to the changes in #3867, the default behavior will now throw when there are actual errors in communication with the down stream services in the serviceList, rather than merely logging to the console and composing a potentially incomplete graph from the services which were available.

Additionally, we've made a lot of other stability improvements to the behavior, which all surfaced in https://github.com/apollographql/apollo-server/blob/master/packages/apollo-gateway/CHANGELOG.md#0140.

I'll close this, but happy to re-address concerns in subsequent follow-ups! Thanks so much for opening this originally!

[krishesk]

Recently we are trying to upgrade our gateway from 0.11.4 to 0.31.1 in order to take advantage of the built-in openTelemetry instrumentation, and this change in the behavior got in our way. I understand the rationale behind the decision, but it's not desirable for everyone, especially in the dev environment, where not all the subgraphs are fully set up and running. With this change, our developers are forced to set up the subgraphs they don't really care in order to start the gateway server, which brings a lot of complaints. Is there a way we can switch this off and go back to how things were prior to this change?