This message was being issued when a new server starts up, prior to ever having
a schema, when a storage secret (or any other artifact) can't be fetched
from GCS (or any error within `updateServiceDefinitions`) despite the fact
that there may be no `previousSchema`.

While I could use `previousSchema` to change the error message, I don't
think that it's this methods concern to decide what happens in the event of
an error.  I think it's only this methods concern to actually do the update,
if it is in fact successful.

Currently, we log the error and just return without throwing which causes
`load` (one of two places where `updateComposition` is called to not
actually fail and follow-up logic then suggests "Gateway successfully loaded
schema", even though that cannot be true.

The nice thing about this change (in addition to allowing someone to `catch`
an error from `ApolloGateway.prototype.load`) is that this should bubble all
the way up to the place where we currently call `load` within Apollo
Server's constructor, and stop the server from starting in this failure
condition:

https://github.com/apollographql/apollo-server/blob/7dcee80ff33061c0911458d593ebbca5a9c73939/packages/apollo-server-core/src/ApolloServer.ts#L366

The other place we call `updateComposition` is within a `setTimeout`.  That
particular case is less troublesome since it will retry on the next interval.

No need to have them so far away from their usage and to even do these
assignments at all when there are already escape routes that exit this code
path.

* Refer to the thing we are processing consistently as "schema definitions".
* Also make them generally more factual.

…an error.

Rely on `message` only if it's present, and fall back to serialization
methods which might exist on the prototype otherwise (e.g. `toJSON`,
`toString`).

Also, switch to a single parameter usage of the logging facility.  While
`console.log` supports it, its certainly possible that the logger will
_not_, and will need that positional parameter for something else.

While successful `updateComposition` invocations were working properly,
failed invocations (including GCS access errors, network hiccups and just
general configuration mistakes) were currently cluttering the logs with
warnings of unhandled promise rejections.

While those unhandled rejections did include the actual error messages, this
adjusts them to be caught and logged in a structured manner with our
`logger`, sparing our logs from those unnecessarily verbose (and scary!)
messages.

Adjusting the typings only works for users of TypeScript.  On the other
hand, this is a one-time assertion which happens at instantiation time which
could save a JavaScript developer from the pain of not knowing what's going
on!

There might be typing improvements to be had, but claiming it as an alternate
approach to handling this isn't correct.  The typings can still be improved,
of course.

Overall, the success/failure behavior should be expected to be similar for
all of these requests, as they all access the same GCS store.

Sometimes, the error object which is being caught here is in fact not a
misconfiguration of server options, but rather an error which was thrown in
a promise chain.

By appending the "Invalid options provided to ApolloServer" string to the
error object's `message` property in this method that is called on _every_
request to the server (`runHttpQuery`), we're risking appending the same
prefix to the same error over and over (i.e. "a: a: a: a: a: b").

While this prefix may have been true at one point, it is no longer possible
to enforce in a world where the schema is obtained asynchronously.

…veries.

Previously, if the initial call to `load` failed, but its intention (fetching a
federated schema configuration in managed federation) is eventually
accomplished via a subsequent fetch (via setInterval polling), the
`executor` would not be set.

This resulted in a continued failure even if the `schema` was eventually set
since federated `schema`s require the Gateway's `executor` to do pull off
their much more complex (remote!) execution strategy!

The solution was simple since `executor` was already present on the actual
`ApolloGateway`, but that required exposing that property as a valid type to
access from the interface that `ApolloGateway` implements: `GraphQLService`.

I don't see why a `GraphQLService` wouldn't have an executor, so it seemed
appropriate to add, particularly since our only `GraphQLService` is the
`ApolloGateway` class itself.

In particular, this blocks any rejected promises which may come from GCS
load prior to returning them to the `schemaDerivedData` promise (which is
where the `initSchema` method assigns the result to).

Failure to do this results in the server middleware sending the error
directly to the client.

…ils.

Another approach to this would be to throw an error here, but this is our
only opportunity to allow the server to recover after initial failure.

On the one hand, this approach is more graceful, but on the other hand,
perhaps we actually want initial failure (after timeouts elapse) to not bind
the other middleware.  Either way, the server doesn't just `process.exit`
right now, and with certain non-Node.js environments, it may be worthwhile
to operate in this mode.

…cts.

Because we want the actual executor to work when a schema might eventually
become available, as it may when `onSchemaChange` hooks eventually succeed.

Co-Authored-By: Trevor Scheer <trevor@apollographql.com>

Co-Authored-By: Trevor Scheer <trevor@apollographql.com>

This reverts commit 4d4ab5b.

To correct the syntax error.

Co-Authored-By: Trevor Scheer <trevor@apollographql.com>

…3856)

Co-authored-by: WhiteSource Renovate <renovatebot@gmail.com>

Ref: #3811 (comment)

No particular reason, but I just didn't enjoy the previous wording (my own!).

These have been released!

…ents