-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements on GraphOS and Fastify integration #946
base: main
Are you sure you want to change the base?
Conversation
d5209eb
to
3eda996
Compare
🚀 Snapshot Release (
|
Package | Version | Info |
---|---|---|
@graphql-tools/batch-delegate |
9.0.35-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/delegate |
10.2.17-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/federation |
3.2.0-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/fusion-runtime |
0.11.9-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway |
1.13.5-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-json |
0.0.4-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-pino |
1.0.0-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/nestjs |
1.0.9-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/plugin-aws-sigv4 |
1.0.6-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/hmac-upstream-signature |
1.2.26-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-opentelemetry |
1.3.53-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-prometheus |
1.3.41-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-runtime |
1.7.0-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/stitch |
9.4.22-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/stitching-directives |
3.1.32-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-common |
0.7.34-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http |
0.6.39-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http-callback |
0.5.26-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-ws |
1.0.9-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/wrap |
10.0.35-alpha-da3ad60bd5595c273b81e55eea65bde564632356 |
npm ↗︎ unpkg ↗︎ |
🚀 Snapshot Release (Node Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared
|
🚀 Snapshot Release (Bun Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared
|
6380a06
to
924921c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the GraphOS and Fastify integration by fixing a bug when the fetcher returns an empty result, enhancing log readability, and adding support for Pino logging and flexible request ID configuration.
- Fix error when fetcher returns an empty result
- Improve log output for readiness checks
- Introduce new Pino logging integration and configurable request IDs
Reviewed Changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated no comments.
File | Description |
---|---|
e2e/graphos-polling/graphos-polling.e2e.ts | Updated the test harness to use Fastify endpoints and fetch-based requests for readiness and GraphQL queries |
e2e/graphos-polling/gateway.config.ts | Removed the configuration file in favor of inline configurations |
.changeset/*.md | Added changeset documentation for various fixes, improvements, and dependency updates |
Files not reviewed (1)
- e2e/graphos-polling/package.json: Language not supported
Comments suppressed due to low confidence (1)
e2e/graphos-polling/graphos-polling.e2e.ts:87
- Consider adding an assertion on the readiness endpoint's response to ensure it meets expected behavior, which would enhance the test coverage of the new readiness checks.
const result$ = fetch(`http://0.0.0.0:${gw.port}/graphql`, {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves GraphOS and Fastify integration with better error handling for empty fetcher results, simplified log readability for readiness checks, and introduces new logging and request ID configuration capabilities.
- Fixes crash when fetcher returns an empty result
- Improves log readability in readiness checks
- Adds Pino logging and flexible request ID configuration
Reviewed Changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated no comments.
File | Description |
---|---|
e2e/graphos-polling/graphos-polling.e2e.ts | Updated polling test with refined gateway creation and use of fetch for readiness and GraphQL endpoints |
e2e/graphos-polling/gateway.config.ts | Removed gateway configuration file, likely replaced by new configuration approaches |
.changeset/* | Updated changesets summarizing the fixes, improvements, and dependency updates |
Files not reviewed (1)
- e2e/graphos-polling/package.json: Language not supported
Comments suppressed due to low confidence (1)
e2e/graphos-polling/graphos-polling.e2e.ts:87
- [nitpick] The variable name 'result$' suggests an observable stream while it holds a Promise; consider renaming it (e.g., 'graphqlResult') to better reflect its asynchronous nature.
const result$ = fetch(`http://0.0.0.0:${gw.port}/graphql`, {
c2b9317
to
b82d9c6
Compare
b82d9c6
to
da3ad60
Compare
let requestLogger = logger; | ||
const requestId = requestIdByRequest.get(request); | ||
if (requestId) { | ||
requestLogger = logger.child({ requestId }); | ||
} | ||
requestLogger.info( | ||
'The operation has been aborted after the supergraph schema reloaded, retrying the operation...', | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that this a temporary workaround waiting for the new Logging system ?
Related GW-270
Documentation -> graphql-hive/console#6663
Fixes
undefined
, the runtime was trying to handle the result then fails with a cryptic errorImprovements
readiness
title and printspasses
orfails
with errors if presentNew Features
Pino integration (also helpful for Fastify integration);
Request ID configuration (helpful for Fastify integration)
By default, first Hive Gateway was checking if
x-request-id
exists in the HTTP headers, then generates and sets a new one.And this can be disabled by setting
requestId
tofalse
in thegatewayConfig
.Now you can configure the request ID generation by providing a function to the
requestId
field in thegatewayConfig
(or inherit from the framework you use).And you can also rename the header name by setting the
headerName
field in thegatewayConfig
.This is useful with Fastify because it handles the request ID generation and propagation by itself.
New Fastify Recipe