From 024f68ad9dbeb10eaa5c17ad1f62f8c725f1d6d7 Mon Sep 17 00:00:00 2001 From: Kamil Kisiela Date: Tue, 10 Oct 2023 14:12:09 +0200 Subject: [PATCH] Include operation name in error message, always finish measureDuration() (#3007) --- .changeset/lemon-students-grow.md | 5 + .changeset/olive-crabs-mix.md | 5 + .changeset/tidy-cheetahs-confess.md | 5 + .../libraries/client/src/internal/usage.ts | 137 +++++++++++++++--- 4 files changed, 133 insertions(+), 19 deletions(-) create mode 100644 .changeset/lemon-students-grow.md create mode 100644 .changeset/olive-crabs-mix.md create mode 100644 .changeset/tidy-cheetahs-confess.md diff --git a/.changeset/lemon-students-grow.md b/.changeset/lemon-students-grow.md new file mode 100644 index 000000000..7a5d7f0c3 --- /dev/null +++ b/.changeset/lemon-students-grow.md @@ -0,0 +1,5 @@ +--- +'@graphql-hive/client': patch +--- + +Finish measuring duration on error diff --git a/.changeset/olive-crabs-mix.md b/.changeset/olive-crabs-mix.md new file mode 100644 index 000000000..abaae3062 --- /dev/null +++ b/.changeset/olive-crabs-mix.md @@ -0,0 +1,5 @@ +--- +'@graphql-hive/client': patch +--- + +Include operation's name in error message diff --git a/.changeset/tidy-cheetahs-confess.md b/.changeset/tidy-cheetahs-confess.md new file mode 100644 index 000000000..39362664c --- /dev/null +++ b/.changeset/tidy-cheetahs-confess.md @@ -0,0 +1,5 @@ +--- +'@graphql-hive/client': patch +--- + +Detect unavailable definition and throw errors with paths pointing to relevant ASTNodes diff --git a/packages/libraries/client/src/internal/usage.ts b/packages/libraries/client/src/internal/usage.ts index 78796ecec..5e263fccb 100644 --- a/packages/libraries/client/src/internal/usage.ts +++ b/packages/libraries/client/src/internal/usage.ts @@ -1,5 +1,6 @@ import { ArgumentNode, + ASTNode, DocumentNode, getNamedType, GraphQLInputType, @@ -13,6 +14,7 @@ import { isInputObjectType, isScalarType, Kind, + NameNode, ObjectFieldNode, OperationDefinitionNode, TypeInfo, @@ -151,25 +153,25 @@ export function createUsage(pluginOptions: HivePluginOptions): UsageCollector { const finish = measureDuration(); return function complete(args, result) { + const duration = finish(); + let providedOperationName: string | undefined = undefined; try { if (isAbortAction(result)) { logger.info(result.reason); - finish(); return; } if (isAsyncIterableIterator(result) || isAsyncIterable(result)) { logger.info('@stream @defer is not supported'); - finish(); return; } - const rootOperation = args.document.definitions.find( + const document = args.document; + const rootOperation = document.definitions.find( o => o.kind === Kind.OPERATION_DEFINITION, ) as OperationDefinitionNode; - const document = args.document; - const operationName = args.operationName || rootOperation.name?.value || 'anonymous'; - const duration = finish(); + providedOperationName = args.operationName || rootOperation.name?.value; + const operationName = providedOperationName || 'anonymous'; if (!excludeSet.has(operationName) && shouldInclude()) { const errors = @@ -206,7 +208,8 @@ export function createUsage(pluginOptions: HivePluginOptions): UsageCollector { }); } } catch (error) { - logger.error(`Failed to collect operation`, error); + const details = providedOperationName ? ` (name: "${providedOperationName}")` : ''; + logger.error(`Failed to collect operation${details}`, error); } }; }, @@ -354,14 +357,34 @@ export function createCollector({ visit( doc, visitWithTypeInfo(typeInfo, { - Field() { - const parent = typeInfo.getParentType()!; - const field = typeInfo.getFieldDef()!; + Field(node, _key, _parent, path, ancestors) { + const parent = typeInfo.getParentType(); + const field = typeInfo.getFieldDef(); + + if (!parent) { + throw new Error( + `Could not find a parent type of a field at ${printPath(path, ancestors, node.name)}`, + ); + } + + if (!field) { + throw new Error( + `Could not find a field definition of a field at ${printPath( + path, + ancestors, + node.name, + )}`, + ); + } markAsUsed(makeId(parent.name, field.name)); }, VariableDefinition(node) { - const inputType = typeInfo.getInputType()!; + const inputType = typeInfo.getInputType(); + + if (!inputType) { + throw new Error(`Could not find an input type of a variable $${node.variable.name}`); + } if (shouldAnalyzeVariableValues) { // Granular collection of variable values is enabled @@ -381,16 +404,53 @@ export function createCollector({ arguments: [], }; }, - Argument(node) { - const parent = typeInfo.getParentType()!; - const field = typeInfo.getFieldDef()!; - const arg = typeInfo.getArgument()!; + Argument(node, _key, _parent, path, ancestors) { + const parent = typeInfo.getParentType(); + const field = typeInfo.getFieldDef(); + const arg = typeInfo.getArgument(); + + if (!parent) { + throw new Error( + `Could not find a parent type of an argument at ${printPath( + path, + ancestors, + node.name, + )}`, + ); + } + + if (!field) { + throw new Error( + `Could not find a field definition of an argument at ${printPath( + path, + ancestors, + node.name, + )}`, + ); + } + + if (!arg) { + throw new Error( + `Could not find an argument definition of an argument at ${printPath( + path, + ancestors, + node.name, + )}`, + ); + } markAsUsed(makeId(parent.name, field.name, arg.name)); collectNode(node); }, - ListValue(node) { - const inputType = typeInfo.getInputType()!; + ListValue(node, _key, _parent, path, ancestors) { + const inputType = typeInfo.getInputType(); + + if (!inputType) { + throw new Error( + `Could not find an input type of a list value at ${printPath(path, ancestors)}`, + ); + } + const inputTypeName = resolveTypeName(inputType); node.values.forEach(value => { @@ -400,8 +460,19 @@ export function createCollector({ } }); }, - ObjectField(node) { - const parentInputType = typeInfo.getParentInputType()!; + ObjectField(node, _key, _parent, path, ancestors) { + const parentInputType = typeInfo.getParentInputType(); + + if (!parentInputType) { + throw new Error( + `Could not find an input type of an object field at ${printPath( + path, + ancestors, + node.name, + )}`, + ); + } + const parentInputTypeName = resolveTypeName(parentInputType); collectNode(node); @@ -445,6 +516,34 @@ function resolveTypeName(inputType: GraphQLType): string { return getNamedType(inputType).name; } +function printPath( + path: readonly (string | number)[], + ancestors: readonly (ASTNode | readonly ASTNode[])[], + leafNameNode?: NameNode, +): string { + const result: string[] = []; + for (let i = 0; i < path.length; i++) { + const key = path[i]; + const ancestor = ancestors[i]; + + if (!ancestor) { + break; + } + + if (key === 'selectionSet') { + if ('name' in ancestor && ancestor.name?.value) { + result.push(ancestor.name.value); + } + } + } + + if (leafNameNode) { + result.push(leafNameNode.value); + } + + return result.join('.'); +} + type GraphQLNamedInputType = Exclude< GraphQLNamedType, GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType