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

RFC: Augment error extensions type #3701

Closed
musjj opened this issue Oct 25, 2024 · 5 comments
Closed

RFC: Augment error extensions type #3701

musjj opened this issue Oct 25, 2024 · 5 comments
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release

Comments

@musjj
Copy link

musjj commented Oct 25, 2024

Summary

GraphQL servers can extend the error type with extensions. An example from this repo:

didAuthError(error) {
return error.graphQLErrors.some(
e => e.extensions?.code === 'UNAUTHORIZED'
);
},

Depending on the server implementation, the shape of the extensions may be uniform across all error responses. In that case, it would be helpful if the client can augment the default Record<string, any> with the actual expected type.

Proposed Solution

Augmenting types can look something like this:

declare module "@urql/core" {
  interface Extensions {
    code: "FORBIDDEN" | "INTERNAL_SERVER_ERROR";
  }
}
@musjj musjj added the future 🔮 An enhancement or feature proposal that will be addressed after the next release label Oct 25, 2024
@JoviDeCroock
Copy link
Collaborator

For our error extensions to be configurable GraphQLJS/GraphQL.web would have to export that as we are using GraphQLError which has its own definition of extensions. The extensions type in @urql/core is used for ExecutionResult extensions and IncrementalPayloadExtensions which are a different part of the spec.

What you are looking to alter is in this part of the spec and would need altering

@musjj
Copy link
Author

musjj commented Nov 1, 2024

Thank you for merging the PR (0no-co/graphql.web#36)! Can this repo also be updated with the new version?

@JoviDeCroock
Copy link
Collaborator

It's a dependency, you can bump this yourself with resolutions

@musjj
Copy link
Author

musjj commented Nov 2, 2024

The latest version does not include the export change: https://www.npmjs.com/package/@0no-co/graphql.web?activeTab=versions. So, graphql.web has to be bumped first.

It looks like that the release workflow hit the rate limit yesterday: https://github.com/0no-co/graphql.web/actions/runs/11626919247/job/32379391261

Can you try re-running it again?

@kitten
Copy link
Member

kitten commented Nov 2, 2024

@musjj: We dont't release changes immediately all the time. Please stop commenting for a release that'll come eventually. The action should release a preview version.

Also all of that said, while I understand why @JoviDeCroock has merged this (it's a trivial addition) you really shouldn't be augmenting this type, since there's no type guarantees around error extensions, and should instead write checks and casts for the extensions type.
There's nothing here that's blocking you ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release
Projects
None yet
Development

No branches or pull requests

3 participants