Skip to content

Commit

Permalink
fix(gatsby): Protect about possibly missing context in graphql (gatsb…
Browse files Browse the repository at this point in the history
…yjs#24108)

* Guard against possibly missing context in resolver

* Warn about bad resolver invocations

* Add warning

* Update packages/gatsby/src/schema/resolvers.ts

Co-authored-by: Ward Peeters <[email protected]>

* Fix typo

Co-authored-by: Ward Peeters <[email protected]>
  • Loading branch information
2 people authored and Kornil committed May 15, 2020
1 parent edadb4a commit 2e8be0a
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 37 deletions.
11 changes: 1 addition & 10 deletions packages/gatsby/src/query/graphql-span-tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -72,13 +73,3 @@ export default class GraphQLSpanTracer implements IGraphQLSpanTracer {
this.activities.set(path.join(`.`), activity)
}
}

function pathToArray(path: Path | undefined): Array<string | number> {
const flattened: Array<string | number> = []
let curr: Path | undefined = path
while (curr) {
flattened.push(curr.key)
curr = curr.prev
}
return flattened.reverse()
}
12 changes: 12 additions & 0 deletions packages/gatsby/src/query/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Path } from "graphql/jsutils/Path"

export const indentString = (string: string): string =>
string.replace(/\n/g, `\n `)

Expand All @@ -8,3 +10,13 @@ export const formatErrorDetails = (errorDetails: Map<string, any>): string =>
${indentString(details.toString())}`
)
.join(`\n`)

export function pathToArray(path: Path | undefined): Array<string | number> {
const flattened: Array<string | number> = []
let curr: Path | undefined = path
while (curr) {
flattened.push(curr.key)
curr = curr.prev
}
return flattened.reverse()
}
14 changes: 7 additions & 7 deletions packages/gatsby/src/schema/infer/__tests__/merge-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
})

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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()
})

Expand Down Expand Up @@ -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)
})
})
})
Expand Down Expand Up @@ -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)
})
})
31 changes: 28 additions & 3 deletions packages/gatsby/src/schema/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import {
SelectionNode,
FieldNode,
} from "graphql"
import { Path } from "graphql/jsutils/Path"
import reporter from "gatsby-cli/lib/reporter"
import { pathToArray } from "../query/utils"
import { getValueAt } from "../utils/get-value-at"
import {
GatsbyResolver,
Expand Down Expand Up @@ -472,7 +475,19 @@ export const defaultFieldResolver: GatsbyResolver<
return null
}

export function tracingResolver<TSource, TArgs>(
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<TSource, TArgs>(
resolver: GatsbyResolver<TSource, TArgs>
): GatsbyResolver<TSource, TArgs> {
return async function wrappedTracingResolver(
Expand All @@ -481,8 +496,18 @@ export function tracingResolver<TSource, TArgs>(
context,
info
): Promise<any> {
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) {
if (context?.tracer) {
activity = context.tracer.createResolverActivity(
info.path,
`${info.parentType.name}.${info.fieldName}`
Expand All @@ -499,4 +524,4 @@ export function tracingResolver<TSource, TArgs>(
}
}

export const defaultTracingResolver = tracingResolver(defaultFieldResolver)
export const defaultResolver = wrappingResolver(defaultFieldResolver)
8 changes: 4 additions & 4 deletions packages/gatsby/src/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ const { addInferredType, addInferredTypes } = require(`./infer`)
const {
findOne,
findManyPaginated,
tracingResolver,
defaultTracingResolver,
wrappingResolver,
defaultResolver,
} = require(`./resolvers`)
const {
processFieldExtensions,
Expand Down Expand Up @@ -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,
})
})
}
Expand Down
26 changes: 13 additions & 13 deletions packages/gatsby/src/schema/types/__tests__/date.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand Down Expand Up @@ -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([
Expand Down

0 comments on commit 2e8be0a

Please sign in to comment.