Mutation response is undefined, but shouldn't be

[extjbhlego]

Intended outcome:
Using Apollo Client and hooks, the typings tell me that the response from an operation will always be defined, but that response.data might not be. As such, we always use response.data?.something when unwrapping the response.

We use the built-in options for onCompleted and onError, and as such, do not wrap our calls in try/catch assuming that any failures in the request will propagate to the onError callback.

Actual outcome:
We get Sentry reports that says TypeError: undefined is not an object (evaluating 'n.data')

How to reproduce the issue:

const response = await ourMutation({
  variables: {
    input: {
      id: someId, 
      otherVar: theValue
    },
  },
});

if (response.data?.ourMutation) {
  setShowSuccess(true);
}

Versions

System:
    OS: macOS 10.15.7
  Binaries:
    Node: 14.15.4 - /usr/local/bin/node
    Yarn: 1.22.10 - ~/Documents/git/MMS-Ticket/node_modules/.bin/yarn
    npm: 6.14.10 - /usr/local/bin/npm
  Browsers:
    Chrome: 88.0.4324.192
    Safari: 14.0.3
  npmPackages:
    @apollo/client: 3.3.6 => 3.3.6 
    apollo: 2.31.0 => 2.31.0 
    react-apollo: 3.1.5 => 3.1.5

[jcreighton]

@extjbhlego Happy to help but we'll need some more information. A runnable reproduction using either this repo or this CodeSandbox will help us debug the issue.

[extjbhlego]

@jcreighton - I've tried, but I cannot find a way to reproduce the problem locally. However, we keep getting reports from Sentry about this error, where TS throws because it tries to read the data property on an undefined response.

The Apollo Client typings says that there will always be a response object when doing mutations (i.e. const response = await myMutation(...)) of type FetchResult<TData>, indicating that it's safe to access the data property.

Therefore, our usage looks like the provided example, doing response.data?..., assuming from the typings that accessing .data will not throw a TypeError, but it does.

I'd gladly try to provide more info and a reproduction case, but I will need some pointers/ideas/direction about where to look or how to approach it. Any help is much appreciated.

[jcreighton]

@extjbhlego Are you still experiencing this issue? If so, could you create a CodeSandbox with the mutation as closely aligned to how it is in your codebase? That could give us a clue as to what is happening.

[extjbhlego]

@jcreighton - Yes, unfortunately.

I created a sandbox indicating our typical way of doing mutations here . It does not reproduce the error, but shows how we use mutations in callbacks, and index into response.data?.[something].

Here is a (slightly censored) screenshot of the error

Two examples from our codebase that throws the error:

if (hasValue(consumeInput)) {
      const response = await mutate({ variables: { input: consumeInput } });
      switch (response.data?.consumeSparePart.__typename) {
}

const handleSubmitTicket = useCallback(async () => {
    if (isInputValid) {
      const response = await createTicket({
        variables: {[our variables]},
      });

      if (response.data?.createTicket) {
        setShowSuccess(true);
      }
   }
}

I hope the above provides enough context for the problem, and if not, please let me know how I can help further demonstrate the issue 😄

[extjbhlego]

@jcreighton - Eureka! 😄

Today, I accidentally stumbled across a way to reproduce the error. It turns out that it can be reproduced if you provide an onError callback to the useMutation config.

I've set up an updated sandbox that produces the issue when you press the button:

https://codesandbox.io/s/late-sound-ettg7?file=/src/App.js

We have a default error handling function that displays a nice default error message for the user in the few cases where we don't handle errors explicitly, which we provide to the onError callback in the useMutation/useQuery configs. From the types and documentation, I would expect that in this case

1. the callback will be invoked
2. response will still be defined as the types indicate, but data will be null, and errors will instead be populated:
   

[extjbhlego]

@jcreighton Can you give any estimate as to when we can expect this to land in a patch? :) Thanks