Skip to content

Commit

Permalink
generate proper errors for invalid context name (#3041)
Browse files Browse the repository at this point in the history
We were triggering an assert in extractSubgraphsFromSupergraph because
we weren't properly detecting an invalid name at validation time. Make
sure regex matches.
  • Loading branch information
clenfest authored Jun 20, 2024
1 parent 0c98eac commit b258406
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
41 changes: 41 additions & 0 deletions composition-js/src/__tests__/compose.setContext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1331,4 +1331,45 @@ describe("setContext tests", () => {
'[Subgraph1] @fromContext argument cannot be used on a field implementing an interface field "I.field".'
);
});

test("invalid context name shouldn't throw", () => {
const subgraph1 = {
name: "Subgraph1",
utl: "https://Subgraph1",
typeDefs: gql`
type Query {
t: T!
}
type T @key(fields: "id") @context(name: "") {
id: ID!
u: U!
prop: String!
}
type U @key(fields: "id") {
id: ID!
field(a: String): Int!
}
`,
};

const subgraph2 = {
name: "Subgraph2",
utl: "https://Subgraph2",
typeDefs: gql`
type Query {
a: Int!
}
type U @key(fields: "id") {
id: ID!
}
`,
};

const result = composeAsFed2Subgraphs([subgraph1, subgraph2]);
expect(result.schema).toBeUndefined();
expect(result.errors?.length).toBe(1);
expect(result.errors?.[0].message).toBe(
'[Subgraph1] Context name "" is invalid. It should have only alphanumeric characters.'
);
});
});
7 changes: 7 additions & 0 deletions internals-js/src/federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1577,12 +1577,19 @@ export class FederationBlueprint extends SchemaBlueprint {
const parent = application.parent;
const name = application.arguments().name as string;

const match = name.match(/^([A-Za-z]\w*)$/);
if (name.includes('_')) {
errorCollector.push(ERRORS.CONTEXT_NAME_INVALID.err(
`Context name "${name}" may not contain an underscore.`,
{ nodes: sourceASTs(application) }
));
}
else if (!match) {
errorCollector.push(ERRORS.CONTEXT_NAME_INVALID.err(
`Context name "${name}" is invalid. It should have only alphanumeric characters.`,
{ nodes: sourceASTs(application) }
));
}
const types = contextToTypeMap.get(name);
if (types) {
types.push(parent);
Expand Down

0 comments on commit b258406

Please sign in to comment.