From 5b33134746eef816a70314fa0689ffb20b1b3dd8 Mon Sep 17 00:00:00 2001 From: Mikhail Novikov Date: Fri, 15 May 2020 10:05:19 +0300 Subject: [PATCH 1/5] Guard against possibly missing context in resolver --- packages/gatsby/src/schema/resolvers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/schema/resolvers.ts b/packages/gatsby/src/schema/resolvers.ts index 6e2cfeb71bf02..62735b4058d24 100644 --- a/packages/gatsby/src/schema/resolvers.ts +++ b/packages/gatsby/src/schema/resolvers.ts @@ -482,7 +482,7 @@ export function tracingResolver( info ): Promise { let activity - if (context.tracer) { + if (context?.tracer) { activity = context.tracer.createResolverActivity( info.path, `${info.parentType.name}.${info.fieldName}` From 5cb24bcb378761ee199a3ebf020713952b769b6d Mon Sep 17 00:00:00 2001 From: Mikhail Novikov Date: Fri, 15 May 2020 10:29:09 +0300 Subject: [PATCH 2/5] Warn about bad resolver invocations --- .../gatsby/src/query/graphql-span-tracer.ts | 11 +------ packages/gatsby/src/query/utils.ts | 12 ++++++++ packages/gatsby/src/schema/resolvers.ts | 29 +++++++++++++++++-- packages/gatsby/src/schema/schema.js | 8 ++--- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/packages/gatsby/src/query/graphql-span-tracer.ts b/packages/gatsby/src/query/graphql-span-tracer.ts index 26071ffdda368..231ac664f56e3 100644 --- a/packages/gatsby/src/query/graphql-span-tracer.ts +++ b/packages/gatsby/src/query/graphql-span-tracer.ts @@ -5,6 +5,7 @@ import { IActivityArgs } from "gatsby-cli/src/reporter/reporter" import { IPhantomReporter } from "gatsby-cli/src/reporter/reporter-phantom" import { IGraphQLSpanTracer } from "../schema/type-definitions" +import { pathToArray } from "./utils" /** * Tracks and knows how to get a parent span for a particular @@ -72,13 +73,3 @@ export default class GraphQLSpanTracer implements IGraphQLSpanTracer { this.activities.set(path.join(`.`), activity) } } - -function pathToArray(path: Path | undefined): Array { - const flattened: Array = [] - let curr: Path | undefined = path - while (curr) { - flattened.push(curr.key) - curr = curr.prev - } - return flattened.reverse() -} diff --git a/packages/gatsby/src/query/utils.ts b/packages/gatsby/src/query/utils.ts index fea157b624a84..d2d9fde767e17 100644 --- a/packages/gatsby/src/query/utils.ts +++ b/packages/gatsby/src/query/utils.ts @@ -1,3 +1,5 @@ +import { Path } from "graphql/jsutils/Path" + export const indentString = (string: string): string => string.replace(/\n/g, `\n `) @@ -8,3 +10,13 @@ export const formatErrorDetails = (errorDetails: Map): string => ${indentString(details.toString())}` ) .join(`\n`) + +export function pathToArray(path: Path | undefined): Array { + const flattened: Array = [] + let curr: Path | undefined = path + while (curr) { + flattened.push(curr.key) + curr = curr.prev + } + return flattened.reverse() +} diff --git a/packages/gatsby/src/schema/resolvers.ts b/packages/gatsby/src/schema/resolvers.ts index 62735b4058d24..f676e0fd0ac4b 100644 --- a/packages/gatsby/src/schema/resolvers.ts +++ b/packages/gatsby/src/schema/resolvers.ts @@ -16,6 +16,9 @@ import { SelectionNode, FieldNode, } from "graphql" +import { Path } from "graphql/jsutils/Path" +const reporter = require(`gatsby-cli/lib/reporter`) +import { pathToArray } from "../query/utils" import { getValueAt } from "../utils/get-value-at" import { GatsbyResolver, @@ -472,7 +475,19 @@ export const defaultFieldResolver: GatsbyResolver< return null } -export function tracingResolver( +let WARNED_ABOUT_RESOLVERS = false +function badResolverInvocationMessage(missingVar: string, path?: Path): string { + const resolverName = path ? `${pathToArray(path)} ` : `` + return `GraphQL Resolver ${resolverName}got called without "${missingVar}" argument. This might cause unexpected errors. + +It's likely that this has happened in a schemaCustomization with manually invoked resolver. If manually invoking resolvers, it's best to invoke them as follows: + + resolve(parent, args, context, info) + +` +} + +export function wrappingResolver( resolver: GatsbyResolver ): GatsbyResolver { return async function wrappedTracingResolver( @@ -481,6 +496,16 @@ export function tracingResolver( context, info ): Promise { + if (!WARNED_ABOUT_RESOLVERS) { + if (!info) { + reporter.warn(badResolverInvocationMessage(`info`)) + WARNED_ABOUT_RESOLVERS = true + } else if (!context) { + reporter.warn(badResolverInvocationMessage(`context`, info.path)) + WARNED_ABOUT_RESOLVERS = true + } + } + let activity if (context?.tracer) { activity = context.tracer.createResolverActivity( @@ -499,4 +524,4 @@ export function tracingResolver( } } -export const defaultTracingResolver = tracingResolver(defaultFieldResolver) +export const defaultResolver = wrappingResolver(defaultFieldResolver) diff --git a/packages/gatsby/src/schema/schema.js b/packages/gatsby/src/schema/schema.js index 1d6e5e2e179de..8b5a63e6f6bf9 100644 --- a/packages/gatsby/src/schema/schema.js +++ b/packages/gatsby/src/schema/schema.js @@ -26,8 +26,8 @@ const { addInferredType, addInferredTypes } = require(`./infer`) const { findOne, findManyPaginated, - tracingResolver, - defaultTracingResolver, + wrappingResolver, + defaultResolver, } = require(`./resolvers`) const { processFieldExtensions, @@ -856,8 +856,8 @@ function attachTracingResolver({ schemaComposer }) { const field = typeComposer.getField(fieldName) typeComposer.extendField(fieldName, { resolve: field.resolve - ? tracingResolver(field.resolve) - : defaultTracingResolver, + ? wrappingResolver(field.resolve) + : defaultResolver, }) }) } From d0cddeea45576bccc5eed57b7f8673127eb0c5b8 Mon Sep 17 00:00:00 2001 From: Mikhail Novikov Date: Fri, 15 May 2020 10:52:17 +0300 Subject: [PATCH 3/5] Add warning --- .../src/schema/infer/__tests__/merge-types.js | 14 +++++----- .../gatsby/src/schema/types/__tests__/date.js | 26 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/gatsby/src/schema/infer/__tests__/merge-types.js b/packages/gatsby/src/schema/infer/__tests__/merge-types.js index 91dc0feb3fa5d..6396478ce7a29 100644 --- a/packages/gatsby/src/schema/infer/__tests__/merge-types.js +++ b/packages/gatsby/src/schema/infer/__tests__/merge-types.js @@ -2,7 +2,7 @@ const { buildObjectType } = require(`../../types/type-builders`) const { store } = require(`../../../redux`) const { build } = require(`../..`) const { actions } = require(`../../../redux/actions`) -const { defaultTracingResolver } = require(`../../resolvers`) +const { defaultResolver } = require(`../../resolvers`) jest.mock(`gatsby-cli/lib/reporter`, () => { return { @@ -286,7 +286,7 @@ describe(`merges explicit and inferred type definitions`, () => { expect(nestedNestedFields.conflict.type.toString()).toBe(`String!`) // Date resolvers - expect(fields.explicitDate.resolve).toBe(defaultTracingResolver) + expect(fields.explicitDate.resolve).toBe(defaultResolver) expect(fields.inferDate.resolve).toBeDefined() }) @@ -337,7 +337,7 @@ describe(`merges explicit and inferred type definitions`, () => { expect(nestedNestedFields.conflict.type.toString()).toBe(`String!`) // Date resolvers - expect(fields.explicitDate.resolve).toBe(defaultTracingResolver) + expect(fields.explicitDate.resolve).toBe(defaultResolver) }) it(`with "infer(noDefaultResolvers: false)"`, async () => { @@ -438,7 +438,7 @@ describe(`merges explicit and inferred type definitions`, () => { expect(nestedNestedFields.conflict.type.toString()).toBe(`String!`) // Date resolvers - expect(fields.explicitDate.resolve).toBe(defaultTracingResolver) + expect(fields.explicitDate.resolve).toBe(defaultResolver) expect(fields.inferDate.resolve).toBeDefined() }) @@ -543,7 +543,7 @@ describe(`merges explicit and inferred type definitions`, () => { expect(nestedNestedFields.conflict.type.toString()).toBe(`String!`) // Date resolvers - expect(fields.explicitDate.resolve).toBe(defaultTracingResolver) + expect(fields.explicitDate.resolve).toBe(defaultResolver) }) }) }) @@ -698,7 +698,7 @@ describe(`merges explicit and inferred type definitions`, () => { const { link, links } = schema.getType(`LinkTest`).getFields() expect(link.type.toString()).toBe(`Test!`) expect(links.type.toString()).toBe(`[Test!]!`) - expect(link.resolve).toBe(defaultTracingResolver) - expect(links.resolve).toBe(defaultTracingResolver) + expect(link.resolve).toBe(defaultResolver) + expect(links.resolve).toBe(defaultResolver) }) }) diff --git a/packages/gatsby/src/schema/types/__tests__/date.js b/packages/gatsby/src/schema/types/__tests__/date.js index 618e35fe6fdb9..4156bf5588d9e 100644 --- a/packages/gatsby/src/schema/types/__tests__/date.js +++ b/packages/gatsby/src/schema/types/__tests__/date.js @@ -2,7 +2,7 @@ const { store } = require(`../../../redux`) const { actions } = require(`../../../redux/actions`) const { build } = require(`../..`) const withResolverContext = require(`../../context`) -const { defaultTracingResolver } = require(`../../resolvers`) +const { defaultResolver } = require(`../../resolvers`) const { isDate, looksLikeADate } = require(`../date`) jest.mock(`gatsby-cli/lib/reporter`, () => { @@ -353,22 +353,22 @@ describe(`dateResolver`, () => { expect(fields.validNanosecond1.resolve).toBeDefined() expect(fields.validNanosecond2.resolve).toBeDefined() // expect(fields.invalidHighPrecision.resolve).toBeDefined() - expect(fields.invalidDate1.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate2.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate3.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate4.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate5.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate6.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate7.resolve).toBe(defaultTracingResolver) + expect(fields.invalidDate1.resolve).toBe(defaultResolver) + expect(fields.invalidDate2.resolve).toBe(defaultResolver) + expect(fields.invalidDate3.resolve).toBe(defaultResolver) + expect(fields.invalidDate4.resolve).toBe(defaultResolver) + expect(fields.invalidDate5.resolve).toBe(defaultResolver) + expect(fields.invalidDate6.resolve).toBe(defaultResolver) + expect(fields.invalidDate7.resolve).toBe(defaultResolver) expect(fields.invalidDate8).toBeUndefined() - expect(fields.invalidDate9.resolve).toBe(defaultTracingResolver) + expect(fields.invalidDate9.resolve).toBe(defaultResolver) expect(fields.invalidDate10).toBeUndefined() - expect(fields.invalidDate11.resolve).toBe(defaultTracingResolver) + expect(fields.invalidDate11.resolve).toBe(defaultResolver) expect(fields.invalidDate12).toBeUndefined() expect(fields.invalidDate13).toBeUndefined() - expect(fields.invalidDate14.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate15.resolve).toBe(defaultTracingResolver) - expect(fields.invalidDate16.resolve).toBe(defaultTracingResolver) + expect(fields.invalidDate14.resolve).toBe(defaultResolver) + expect(fields.invalidDate15.resolve).toBe(defaultResolver) + expect(fields.invalidDate16.resolve).toBe(defaultResolver) }) it.each([ From 9a82b231089443e2a035583bb82d985a1a95591c Mon Sep 17 00:00:00 2001 From: Mikhail Novikov Date: Fri, 15 May 2020 11:04:29 +0300 Subject: [PATCH 4/5] Update packages/gatsby/src/schema/resolvers.ts Co-authored-by: Ward Peeters --- packages/gatsby/src/schema/resolvers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/schema/resolvers.ts b/packages/gatsby/src/schema/resolvers.ts index f676e0fd0ac4b..5cbac8be846a8 100644 --- a/packages/gatsby/src/schema/resolvers.ts +++ b/packages/gatsby/src/schema/resolvers.ts @@ -17,7 +17,7 @@ import { FieldNode, } from "graphql" import { Path } from "graphql/jsutils/Path" -const reporter = require(`gatsby-cli/lib/reporter`) +import reporter from "gatsby-cli/lib/reporter`) import { pathToArray } from "../query/utils" import { getValueAt } from "../utils/get-value-at" import { From 4efc54a946f993d2ef8ee79e8b4d1002a4282908 Mon Sep 17 00:00:00 2001 From: Mikhail Novikov Date: Fri, 15 May 2020 11:14:36 +0300 Subject: [PATCH 5/5] Fix typo --- packages/gatsby/src/schema/resolvers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/schema/resolvers.ts b/packages/gatsby/src/schema/resolvers.ts index 5cbac8be846a8..7b81a6cf774d2 100644 --- a/packages/gatsby/src/schema/resolvers.ts +++ b/packages/gatsby/src/schema/resolvers.ts @@ -17,7 +17,7 @@ import { FieldNode, } from "graphql" import { Path } from "graphql/jsutils/Path" -import reporter from "gatsby-cli/lib/reporter`) +import reporter from "gatsby-cli/lib/reporter" import { pathToArray } from "../query/utils" import { getValueAt } from "../utils/get-value-at" import {