Skip to content

Commit

Permalink
Preset Unknown types errors as GraphQLError (#68)
Browse files Browse the repository at this point in the history
Previously, some logic paths were throwing an exception
  • Loading branch information
kamilkisiela authored Jul 15, 2024
1 parent d097a2d commit 51dd57a
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .changeset/sour-adults-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@theguild/federation-composition': patch
---

Unknown types are now always reported as GraphQLError (previously in some logic paths, it was an
exception).
33 changes: 33 additions & 0 deletions __tests__/subgraph/key-fields.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,37 @@ testVersions((api, version) => {
]),
);
});

test('missing _FieldSet definition', () => {
expect(
api.composeServices([
{
name: 'foo',
typeDefs: graphql`
directive @key(fields: _FieldSet!) repeatable on OBJECT | INTERFACE
type Query {
users: [User]
}
type User @key(fields: "id") {
id: ID!
name: String!
}
`,
},
]),
).toEqual(
expect.objectContaining({
errors: expect.arrayContaining([
expect.objectContaining({
message: expect.stringContaining(`[foo] Unknown type _FieldSet`),
extensions: expect.objectContaining({
code: 'INVALID_GRAPHQL',
}),
}),
]),
}),
);
});
});
44 changes: 36 additions & 8 deletions src/subgraph/validation/validate-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const specifiedScalars = new Set(specifiedScalarTypes.map(t => t.name));

type ReportErrorFn = (message: string) => void;

const SKIP = Symbol('skip');

export function validateSubgraphState(state: SubgraphState, context: SubgraphValidationContext) {
const errors: GraphQLError[] = [];

Expand Down Expand Up @@ -130,7 +132,13 @@ function validateDirectives(
continue;
}

if (!isInputType(state, argInputTypeName)) {
const isInput = isInputType(state, argInputTypeName);

if (isInput === SKIP) {
continue;
}

if (!isInput) {
reportError(
`The type of @${directive.name}(${arg.name}:) must be Input Type ` +
`but got: ${arg.type}.`,
Expand Down Expand Up @@ -255,7 +263,13 @@ function validateFields(
}

// Ensure the type is an output type
if (!isOutputType(state, fieldTypeName)) {
const isOutput = isOutputType(state, fieldTypeName);

if (isOutput === SKIP) {
continue;
}

if (!isOutput) {
reportError(
`The type of "${type.name}.${field.name}" must be Output Type but got: "${field.type}".`,
);
Expand All @@ -278,7 +292,13 @@ function validateFields(
}

// Ensure the type is an input type
if (!isInputType(state, argTypeName)) {
const isInput = isInputType(state, argTypeName);

if (isInput === SKIP) {
continue;
}

if (!isInput) {
const isList = arg.type.endsWith(']');
const isNonNull = arg.type.endsWith('!');
const extra = isList ? ', a ListType' : isNonNull ? ', a NonNullType' : '';
Expand Down Expand Up @@ -459,7 +479,13 @@ function validateInputFields(
}

// Ensure the type is an input type
if (!isInputType(state, fieldTypeName)) {
const isInput = isInputType(state, fieldTypeName);

if (isInput === SKIP) {
continue;
}

if (!isInput) {
const isList = field.type.endsWith(']');
const isNonNull = field.type.endsWith('!');
const extra = isList ? ', a ListType' : isNonNull ? ', a NonNullType' : '';
Expand Down Expand Up @@ -766,7 +792,7 @@ function isRequiredInputField(arg: InputField): boolean {
return isNonNull(arg.type) && arg.defaultValue === undefined;
}

function isOutputType(state: SubgraphState, typeName: string): boolean {
function isOutputType(state: SubgraphState, typeName: string) {
const type = state.types.get(typeName);

if (!type) {
Expand All @@ -776,15 +802,16 @@ function isOutputType(state: SubgraphState, typeName: string): boolean {

// The existence of the type was already validated.
// See: KnownTypeNamesRule
throw new Error(`Expected to find ${typeName} type`);
// No need to throw an exception here, because it affects the rest of the validation logic.
return SKIP;
}

// Only input types are not output types (scalars and enums are in both).
// This is a quick check, to make it least expensive possible.
return !isInputObjectType(type);
}

export function isInputType(state: SubgraphState, typeName: string): boolean {
export function isInputType(state: SubgraphState, typeName: string) {
const type = state.types.get(typeName);

if (!type) {
Expand All @@ -794,7 +821,8 @@ export function isInputType(state: SubgraphState, typeName: string): boolean {

// The existence of the type was already validated.
// See: KnownTypeNamesRule
throw new Error(`Expected to find ${typeName} type`);
// No need to throw an exception here, because it affects the rest of the validation logic.
return SKIP;
}

return isScalarType(type) || isEnumType(type) || isInputObjectType(type);
Expand Down

0 comments on commit 51dd57a

Please sign in to comment.