-
Notifications
You must be signed in to change notification settings - Fork 580
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
fix(apollo-usage-report): wait for hashing schema before preparing the report #3647
Conversation
🦋 Changeset detectedLatest commit: cf3fb38 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Apollo Federation Subgraph Compatibility Results
Learn more: |
💻 Website PreviewThe latest changes are available as preview in: https://246e249b.graphql-yoga.pages.dev |
✅ Benchmark Results
|
@@ -227,7 +222,9 @@ export function useApolloInstrumentation(options: ApolloInlineTracePluginOptions | |||
const reqCtx = ctxForReq.get(request); | |||
if (!reqCtx) return; | |||
// onResultProcess will be called only once since we disallow async iterables | |||
if (reqCtx.stopped) throw new Error('Trace stopped multiple times'); | |||
if (reqCtx.stopped) { | |||
logger.debug('Trace stopped multiple times'); |
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.
perhaps at least a warning ? This is probably not normal to have :-P
@@ -167,7 +174,8 @@ export function useApolloUsageReport(options: ApolloUsageReportOptions = {}): Pl | |||
|
|||
for (const trace of reqCtx.traces.values()) { | |||
if (!trace.schemaId || !trace.operationKey) { | |||
throw new TypeError('Misformed trace, missing operation key or schema id'); | |||
logger.debug('Misformed trace, missing operation key or schema id'); |
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.
Same here, we should probably at least have a warning ?
Wait for setting schemaId to prevent race condition, previously it wasn't waiting so sometimes at the beginning,
Misformed trace, missing operation key or schema id
was thrown