Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds federation error code coverage docs #61

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

PascalSenn
Copy link
Contributor

No description provided.

Copy link

linux-foundation-easycla bot commented Nov 13, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

| 🔴 | INVALID_LINK_IDENTIFIER | A URL/version for a @link feature is invalid/does not respect the specification. |
| 🔴 | INVALID_SUBGRAPH_NAME | A subgraph name is invalid. (Subgraph names cannot be a single underscore (\*)). |
| 🔴 | LINK_IMPORT_NAME_MISMATCH | The import name for a merged directive (as declared by the relevant @link(import:) argument) is inconsistent between subgraphs. |
| 🔴 | REQUIRES_FIELDS_MISSING_EXTERNAL | The fields argument of a @requires directive includes a field that is not marked as @external. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend schema
              @link(
                url: "https://specs.apollo.dev/federation/v2.3"
                import: ["@key", "@requires"]
              )

            type Query {
              users: [User]
            }

            type User @key(fields: "id") {
              id: ID!
              internalId: ID!
              profile: Profile @requires(fields: "internalId")
            }

            type Profile {
              name: String
            }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[users] On field "User.profile", for @requires(fields: "internalId"): field "User.internalId" should not be part of a @requires since it is already provided by this subgraph (it is not marked @external)

| 🔴 | INVALID_SUBGRAPH_NAME | A subgraph name is invalid. (Subgraph names cannot be a single underscore (\*)). |
| 🔴 | LINK_IMPORT_NAME_MISMATCH | The import name for a merged directive (as declared by the relevant @link(import:) argument) is inconsistent between subgraphs. |
| 🔴 | REQUIRES_FIELDS_MISSING_EXTERNAL | The fields argument of a @requires directive includes a field that is not marked as @external. |
| 🔴 | REQUIRES_UNSUPPORTED_ON_INTERFACE | A @requires directive is used on an interface, which is not (yet) supported. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend schema
              @link(
                url: "https://specs.apollo.dev/federation/v2.3"
                import: ["@key", "@requires"]
              )

            type Query {
              users: [User]
            }

            type RegisteredUser implements User @key(fields: "id") {
              id: ID!
              name: String!
              email: String
            }

            interface User @key(fields: "id") {
              id: ID!
              name: String!
              email: String @requires(fields: "name")
            }
[users] Cannot use @requires on field "User.email" of parent type "User": @requires is not yet supported within interfaces

| Status | Error | Description |
| ------ | ----------------------------------------- | ------------------------------------------------------------------------------------------------------ |
| ❓ | EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE | The @external directive collides with other directives in some situations. |
| ❓ | INTERFACE_OBJECT_USAGE_ERROR | Error in the usage of the @interfaceObject directive. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# subgraph book

extend schema @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"])

            interface Media @key(fields: "id") {
              id: ID!
              title: String!
            }

            type Book implements Media @key(fields: "id") {
              id: ID!
              title: String!
            }
            
# subgraph review

extend schema
              @link(
                url: "https://specs.apollo.dev/federation/v2.3"
                import: ["@key", "@interfaceObject"]
              )

            type Media @interfaceObject {
              id: ID!
              reviews: [Review!]!
            }

            type Review {
              score: Int!
            }

            type Query {
              topRatedMedia: [Media!]!
            }
The @interfaceObject directive can only be applied to entity types but type "Media" has no @key in this subgraph.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or

# subgraph a
extend schema
                  @link(
                    url: "https://specs.apollo.dev/federation/v2.3"
                    import: ["@key", "@interfaceObject"]
                  )

                type Query {
                  hello: String
                }

# subgraph b
extend schema
                  @link(
                    url: "https://specs.apollo.dev/federation/v2.3"
                    import: ["@key", "@interfaceObject"]
                  )
                type Query {
                  otherField: String
                }

                type MyInterface @key(fields: "id") @interfaceObject {
                  id: ID!
                  newField: String
                }                
                ```
                

Type "MyInterface" is declared with @interfaceObject in all the subgraphs in which is is defined (it is defined in subgraph "b" but should be defined as an interface in at least one subgraph)

| ❓ | EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE | The @external directive collides with other directives in some situations. |
| ❓ | INTERFACE_OBJECT_USAGE_ERROR | Error in the usage of the @interfaceObject directive. |
| ❓ | INVALID_FEDERATION_SUPERGRAPH | Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema. |
| ❓ | KEY_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @key directive is not a string. |
Copy link

@kamilkisiela kamilkisiela Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend schema @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"])

            type Query {
              users: [User!]!
            }

            type User @key(fields: true) {
              id: ID
            }
[users] On type "User", for @key(fields: true): Invalid value for argument "fields": must be a string.

| ❓ | INTERFACE_OBJECT_USAGE_ERROR | Error in the usage of the @interfaceObject directive. |
| ❓ | INVALID_FEDERATION_SUPERGRAPH | Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema. |
| ❓ | KEY_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @key directive is not a string. |
| ❓ | MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL | In a subgraph, a field is both marked @external and has a merged directive applied to it. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few examples, I can provide them if needed, but those are relevant errors messages:

[users] Cannot apply merged directive @inaccessible to external field "User.internalId"

[users] Cannot apply merged directive @tag(name: "public") to external field "User.internalId"

| ❓ | INVALID_FEDERATION_SUPERGRAPH | Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema. |
| ❓ | KEY_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @key directive is not a string. |
| ❓ | MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL | In a subgraph, a field is both marked @external and has a merged directive applied to it. |
| ❓ | PROVIDES_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @provides directive is not a string. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend schema
              @link(
                url: "https://specs.apollo.dev/federation/v2.3"
                import: ["@key", "@provides"]
              )

            type Payment @key(fields: "id") {
              id: ID!
              amount: Int!
            }

            type Invoice @key(fields: "id") {
              id: ID!
              amount: Int!
              payment: Payment @provides(fields: true)
            }
[billing] On field "Invoice.payment", for @provides(fields: true): Invalid value for argument "fields": must be a string.

| ❓ | KEY_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @key directive is not a string. |
| ❓ | MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL | In a subgraph, a field is both marked @external and has a merged directive applied to it. |
| ❓ | PROVIDES_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @provides directive is not a string. |
| ❓ | PROVIDES_ON_NON_OBJECT_FIELD | A @provides directive is used to mark a field whose base type is not an object type. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend schema
              @link(
                url: "https://specs.apollo.dev/federation/v2.3"
                import: ["@key", "@provides"]
              )

            type Query {
              users: [User]
            }

            type RegisteredUser implements User @key(fields: "id") {
              id: ID!
              name: String!
              email: String
            }

            interface User @key(fields: "id") {
              id: ID!
              name: String!
              email: String @provides(fields: "name")
            }
[users] Invalid @provides directive on field "User.email": field has type "String" which is not a Composite Type

| ❓ | MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL | In a subgraph, a field is both marked @external and has a merged directive applied to it. |
| ❓ | PROVIDES_INVALID_FIELDS_TYPE | The value passed to the fields argument of a @provides directive is not a string. |
| ❓ | PROVIDES_ON_NON_OBJECT_FIELD | A @provides directive is used to mark a field whose base type is not an object type. |
| ❓ | PROVIDES_UNSUPPORTED_ON_INTERFACE | A @provides directive is used on an interface, which is not (yet) supported. |
Copy link

@kamilkisiela kamilkisiela Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend schema
              @link(
                url: "https://specs.apollo.dev/federation/v2.1"
                import: ["@key", "@provides"]
              )

            type Query {
              users: [User]
            }

            type RegisteredUser implements User @key(fields: "id") {
              id: ID!
              name: String!
              profile: Profile
            }

            interface User @key(fields: "id") {
              id: ID!
              name: String!
              profile: Profile @provides(fields: "name")
            }

            type Profile {
              name: String
            }
[users] Cannot use @provides on field "User.profile" of parent type "User": @provides is not yet supported within interfaces

@kamilkisiela
Copy link

I can provide more examples and cover more rules, just say so :)


**Explanatory Text**

This rule enforces that, for any subgraph schema, if a root mutation type is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This rule enforces that, for any subgraph schema, if a root mutation type is
This rule enforces that, for any source schema, if a root mutation type is

Comment on lines +155 to +162
type Mutation {
createProduct(name: String): Product
}

type Product {
id: ID!
name: String
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example is unnecessarily verbose. There's no need for an object type here.

explicitly named `Mutation` creates inconsistencies in schema design and
violates the composite schema specification.

Valid Example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Valid Example:
Valid example:


**Explanatory Text**

This rule enforces that the root query type in any subgraph schema must be named
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This rule enforces that the root query type in any subgraph schema must be named
This rule enforces that the root query type in any source schema must be named


**Examples**

Valid Example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Valid Example:
Valid example:


**Explanatory Text**

This rule enforces that, for any subgraph schema, if a root subscription type is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This rule enforces that, for any subgraph schema, if a root subscription type is
This rule enforces that, for any source schema, if a root subscription type is


**Examples**

Valid Example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Valid Example:
Valid example:

Valid Example:

```graphql example
# Subgraph A
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Subgraph A

Comment on lines +288 to +295
type Subscription {
productCreated: Product
}

type Product {
id: ID!
name: String
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example is unnecessarily verbose. There's no need for an object type here.

defined.

```graphql counter-example
# Subgraph A
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Subgraph A

- Let {types} be the set of all object types that are annotated with the
`@key` directive in {schema}.
- For each {type} in {types}:
- Let {keyFields} be the set of fields referenced by the `fields` argument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be multiple key directives.

**Formal Specification**

- Let {schema} be the set of all source schemas.
- Let {types} be the set of all object types that are annotated with the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or interface types.

}
```

In this following counter example, the `@key` directive references a field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In this following counter example, the `@key` directive references a field
In this counter example, the `@key` directive references a field

Comment on lines +333 to +334
- Let {keyFields} be the set of fields referenced by the `fields` argument
of the `@key` directive on {type}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including nested fields, right?

```graphql counter-example
type Product @key(fields: "featuredItem { id }") {
featuredItem: Node!
sku: String!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing this field, since it's not relevant to the example.

```graphql counter-example
type Product @key(fields: "tags") {
tags: [String!]!
sku: String!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing this field, since it's not relevant to the example.

```graphql counter-example
type Product @key(fields: "relatedItems") {
relatedItems: Related!
sku: String!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing this field, since it's not relevant to the example.

- For each {type} in {types}:
- Let {fields} be the string value of the `fields` argument of the `@key`
directive on {type}.
- For each {selection} in {fields}:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iterating over a string (fields) would give you a character. I think that there's a missing step here (parsing the fields to a selection set). It should also be recursive.

Comment on lines +434 to +435
directive @lowercase on FIELD_DEFINITION

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
directive @lowercase on FIELD_DEFINITION

**Examples**

In this example, the `User` type has a valid `@key` directive that references
the argument free fields `id` and `name`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the argument free fields `id` and `name`.
the argument-free fields `id` and `name`.

- Let {types} be the set of all object types that are annotated with the
`@key` directive in {schema}.
- For each {type} in {types}:
- Let {fields} be the set of string values of the `fields` argument of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, not really just a "set of string values", but a parsed selection set (that can have child selections).

Comment on lines +570 to +571
1. It must have valid GraphQL syntax.
2. It must reference fields that are defined on the annotated type.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these should be separate rules, since the messages would be different, one about invalid syntax, and one about an invalid reference?

@PascalSenn PascalSenn changed the title Adds federation error code coerage docs Adds federation error code coverage docs Dec 27, 2024
Comment on lines +659 to +660
directive @lowercase on FIELD_DEFINITION

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
directive @lowercase on FIELD_DEFINITION

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants