Release 2.12.0 #3855
Merged
Release 2.12.0 #3855
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I encountered this when trying to land #3553, which updates to DataLoader v2. These type errors were previous masked by a scope problem which resulted in `this.loader` being `any`-typed inside the callback that's passed to `DataLoader`'s first parameter (`batchLoadFn`). Fixing that by leveraging the locally scoped variable revealed that the type for the loader instance itself is incorrect since `mget` will return `null` for a key if it is not present in the cache. This adjusts the typings to fix both of those.
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.
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 <[email protected]>
Co-Authored-By: Trevor Scheer <[email protected]>
This reverts commit 4d4ab5b.
To correct the syntax error. Co-Authored-By: Trevor Scheer <[email protected]>
|
May I bring more attention to #3508? |
No particular reason, but I just didn't enjoy the previous wording (my own!).
- [email protected] - [email protected] - @apollo/[email protected] - @apollo/[email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]
Implement gateway retry logic for requests to GCS. Failed requests will retry up to 5 times. Additionally, this PR adjusts how polling is done in order to prevent the possibility of multiple in-flight updates. The next "tick" only begins after a full round of updating is completed rather than on a perfectly regular interval. Thanks to @abernix for suggesting this change. For more details please see the PR description.
The `any`-typed errors don't provide as much of a safety net as an `Error` type would, any `messsage` was misspelled!
* plugins: Declare types rather than use in-line types for life-cycle hooks. This makes it easier to reason about the types and to keep things DRY, in addition to preserving a more real stacking of types as the stages of server life-cycles progress. E.g., `validationDidStart` will have a super-set of `Required` properties which were present in the `parsingDidStart` phase. Now, rather than actually repeating them verbosely, they will be intersected. These new explicit types will live in `apollo-server-types` where they can also be relied upon by `@apollo/gateway` and other packages, but they'll also be exported directly from the `apollo-server-plugin-base` package. Reason being: The overall concept of `apollo-server-types` and this `apollo-server-plugin-base` package is that they should NOT depend directly on "core", in order to avoid close coupling of plugin support and specific server versions. In other words, someone should be able to install a version of a plugin that depends on `apollo-server-plugin-base` in their application which uses a semver compatible plugin API but NOT the same semver range of `apollo-server-core`, though they are currently similar. They are duplicated concepts right now where one package is intended to be for public plugin exposure (`-plugin-base`), while the other (`-types`) is meant to be used internally. In the future, `apollo-server-types` and `apollo-server-plugin-base` will probably roll into the same "types" package, but that is not today! * plugins: Same as `source`, `queryHash` is required at `parsingDidStart`. They are defined at the same time, as noted in the referenced link. Ref: https://github.com/apollographql/apollo-server/blob/eadcc6b4/packages/apollo-server-core/src/requestPipeline.ts#L196-L197 * Remove unnecessary casting. Per: https://github.com/apollographql/apollo-server/pull/3902/files#r395767682 Thanks to: d1b7a82
- [email protected] - [email protected] - [email protected] - @apollo/[email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]
This will make sure to bump the releases of both federation and server to ensure that the version bumps stay aligned during the prerelease, and avoiding some occasionally bumping to `alpha.2` while others progress to `alpha.3` (for example). This is only a concern for documentation (e.g. CHANGELOG) and human approachability, not for anything technical. cc @trevor-scheer
- [email protected] - [email protected] - [email protected] - @apollo/[email protected] - @apollo/[email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]
We currently have two imports for `graphql`, one which is using named exports, and another which imports "*" - which would also get all of those named imports. I'm sure that the named imports are just an artifact of VSCode auto-importing, but let's just use the named imports for their specificity and clarity as well.
This commit introduces a new configuration option to the gateway: performServiceHealthChecks. This option adds an additional
precautionary step to the gateway load and schema update code paths.
A gateway with this setting turned on will send a simple graphql query
to all downstream services ({ __typename }) to ensure the service is
live and responsive. It performs these queries on initial schema load
as well as during a schema update. The interesting failure behaviors are:
* On load: throw an error, failure to load.
* On schema update: "rollback" (never roll forward). Log the event, but continue serving requests with the old schema.
In response to the release of [email protected], this updates all peerDependencies in the apollo-server monorepo to include the latest version of graphql-js. Note: this drops support for the previously supported rcs and will result in a peerDependencies warning if you choose not to upgrade your version of graphql from an rc to latest.
- [email protected] - [email protected] - [email protected] - @apollo/[email protected] - @apollo/[email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]
This is a work-around, but should not preclude or prevent Node.js 6 from working at runtime, since this is a failure in the local repository's ability to generate the `proto` file for publishing in `apollo-engine-reporting-protobuf`, not a limitation of the generated file (which will be published). Essentially, the release is being prevented since the Node.js workflow is failing because `@apollo/protobufjs`'s insistence on not having lockstep versioning or being constrained by a `package-lock.json`. This results in an unexpected update to a transitive dependency (`mkdirp`) which is caused by `mkdirp` recently changing its `latest` tag to point to the `v1.x.x` release line rather than the `0.5.x` release line it had been using 4 days ago. cc @trevor-scheer
- [email protected] - [email protected] - [email protected] - @apollo/[email protected] - @apollo/[email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As with release PRs in the past, this is a PR tracking a
release-x.y.zbranch for an upcoming release of Apollo Server. 🙌 The version in the title of this PR should correspond to the appropriate branch.Check the appropriate milestone (to the right) for more details on what we hope to get into this release!
The intention of these release branches is to gather changes which are intended to land in a specific version (again, indiciated by the subject of this PR). Release branches allow additional clarity into what is being staged, provide a forum for comments from the community pertaining to the release's stability, and to facilitate the creation of pre-releases (e.g. alpha, beta, rc) without affecting the master branch.
PRs for new features might be opened against or re-targeted to this branch by the project maintainers. The master branch may be periodically merged into this branch up until the point in time that this branch is being prepared for release. Depending on the size of the release, this may be once it reaches RC (release candidate) stage with an -rc.x release suffix. Some less substantial releases may be short-lived and may never have pre-release versions.
When this version is officially released onto the latest npm tag, this PR will be merged into master.