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

Error types don't match actual error data #6309

Closed
dmarkow opened this issue May 19, 2020 · 3 comments · Fixed by #6319
Closed

Error types don't match actual error data #6309

dmarkow opened this issue May 19, 2020 · 3 comments · Fixed by #6319
Labels
📚 good-first-issue Issues that are suitable for first-time contributors. 🙏 help-wanted
Milestone

Comments

@dmarkow
Copy link
Contributor

dmarkow commented May 19, 2020

Presumably, the error that apollo-client raises and the error passed into apollo-link-error are the same. However, the networkError keys don't match up. apollo-link-error correctly uses Error | ServerError | ServerParseError whereas apollo-client just uses Error.

This means that accessing things like networkError.statusCode isn't possible on the error raised by apollo-client without casting it to a different type first.

// apollo-link-error/src/index.ts

import { ServerError, ServerParseError } from 'apollo-link-http-common';

export interface ErrorResponse {
  graphQLErrors?: ReadonlyArray<GraphQLError>;
  networkError?: Error | ServerError | ServerParseError;
  response?: ExecutionResult;
  operation: Operation;
  forward: NextLink;
}

// apollo-client/src/errors/ApolloError.ts

export class ApolloError extends Error {
  public message: string;
  public graphQLErrors: ReadonlyArray<GraphQLError>;
  public networkError: Error | null;

  ...
}

Intended outcome:
Be able to access things like networkError?.statusCode on the error object.

Actual outcome:
networkError is typed as Error | null so you can't access status codes, etc.

How to reproduce the issue:

I ran into this by trying to work with the error object:

const [mutate, { error }] = useMutation(myMutation);
console.log(error?.networkError?.statusCode) // Property 'statusCode' does not exist on type 'Error'.

Versions
System:
OS: macOS 10.15.3
Binaries:
Node: 12.16.2 - ~/.volta/tools/image/node/12.16.2/6.14.4/bin/node
Yarn: 1.22.4
npm: 6.14.4 - ~/.volta/tools/image/node/12.16.2/6.14.4/bin/npm
Browsers:
Chrome: 81.0.4044.138
Safari: 13.0.5
npmPackages:
@apollo/react-hooks: 3.1.5 => 3.1.5
apollo: ^2.27.4 => 2.27.4
apollo-cache: ^1.3.5 => 1.3.5
apollo-cache-inmemory: 1.6.6 => 1.6.6
apollo-client: 2.6.10 => 2.6.10
apollo-fetch: ^0.7.0 => 0.7.0
apollo-link: ^1.2.14 => 1.2.14
apollo-link-batch-http: ^1.2.14 => 1.2.14
apollo-link-error: ^1.1.13 => 1.1.13
apollo-link-http: ^1.5.17 => 1.5.17
apollo-utilities: ^1.3.4 => 1.3.4
react-apollo: 3.1.5 => 3.1.5

@hwillson hwillson added this to the Release 3.0 milestone May 19, 2020
@hwillson hwillson added 📚 good-first-issue Issues that are suitable for first-time contributors. 🙏 help-wanted labels May 19, 2020
@hwillson
Copy link
Member

Thanks for pointing this out @dmarkow. If you (or anyone else) is interested in submitting a PR to address this in the AC 3 codebase (master of this repo), we'll get it in. Thanks!

@dmarkow
Copy link
Contributor Author

dmarkow commented May 19, 2020

@hwillson apollo-link-http has a dependency on apollo-link-http-common to provide the ServerError and ServerParseError types. Would it be preferable to introduce the same dependency in apollo-client or to just paste in the types?

@hwillson
Copy link
Member

@dmarkow AC3 should already have ServerError and ServerParseError types in the codebase (although they are a bit all over the place):

If you can use those types, that would be great - thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📚 good-first-issue Issues that are suitable for first-time contributors. 🙏 help-wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants