Return null from readQuery when data is not in store

[MaxDesiatov]

How to reproduce the issue:

const data = proxy.readQuery({query: gql`{ notInTheStore }`})

Intended outcome:

Returns null

Actual outcome:

Throws an error: ERROR Error: Uncaught (in promise): Error: Can't find field notInTheStore on object (ROOT_QUERY) undefined.

Migrated from:
apollographql/apollo-client#2007
apollographql/apollo-client#1542

[bbugh]

Longer discussion of this on a closed task (it is still a problem) apollographql/apollo-client#1542

[lorensr]

Thanks @bbugh. Updated title & description—it looks like returning null is the favored behavior.

I'm for labeling help-wanted

[bbugh]

The consensus does seem to be to return null, but I think perhaps undefined is more correct for this use case. It's reasonable to expect that a valid readQuery would return a null value if the query is found and the value is actually null. I expect this is why the throw is there in the first place. Thus, I think the return value when a query isn't found should be undefined. Here's some silly code to demonstrate:

const query = gql`{ thing { name } }`
const data = { thing: { name: null } }

// Write a null value to the cache
store.writeQuery({ query, data })

// found query on readQuery, so it should return the value
store.readQuery(query)
=> null  // this is *actually* null

// proposal for null: doesn't make sense to return null because the query doesn't exist
// no way to tell if this is actually a real value OR the query failed
store.readQuery(gql`{ missingQuery { id } }`)
=> null  // this isn't null, it's missing

// proposal for undefined: query doesn't exist so it's not defined
store.readQuery(gql`{ missingQuery { id } }`)
=> undefined // correct - it's not actually defined

[nicky-lenaers]

Despite this being quite an annoying bug that forces us into catching the error returned from readQuery, I've created a minimal workaround to ease the pain and centrally catch the error:

import { Injectable } from '@angular/core';
import { DataProxy } from 'apollo-cache';

@Injectable({
  providedIn: 'root'
})
export class GraphQLService {

  public readQuery<T>(proxy: DataProxy, query: any): T | undefined {
    try {
      const data = proxy.readQuery<T>({ query });
      return data;
    } catch {
      return undefined;
    }
  }
}

You can consume this service anywhere and use it to "safely" use readQuery. For now.

[mcdougal]

I have also been using a workaround (albeit a pretty ugly one). The code below has allowed me to call readQuery the normal way but it will trap for the error and return undefined. My hope is if the behavior suggested in this issue ever makes it in to Apollo I can just remove these handful of lines without touching the rest of the codebase.

  const cache = new InMemoryCache();

  // TODO: Monkey-patching in a fix for an open issue suggesting that
  // `readQuery` should return null or undefined if the query is not yet in the
  // cache: https://github.com/apollographql/apollo-feature-requests/issues/1
  cache.originalReadQuery = cache.readQuery;
  cache.readQuery = (...args) => {
    try {
      return cache.originalReadQuery(...args);
    } catch (err) {
      return undefined;
    }
  };

  return new ApolloClient({
    link,
    cache,
  });

[seanharr11]

I'd like to contend that this is a bug and not a feature-request.

The type definition of readQuery suggests that the method can return null, which makes it even more unexpected that we get an exception. If I query a resource in a datastore (i.e. SELECT * FROM foo), by default I get an empty collection back, not a DatabaseError.

Further, if the official apollo docs suggest that the update() method is the:

"...recommended way of updating the cache after a query"

...then we really should have expected, intuitive & tight behavior here. This bug opens up a whole new class of runtime errors that won't be found until production.

These errors occur quite frequently as new features are rolled out in production (lots of empty collections throughout the app's state!). When a user creates a new resource in a collection, but has never "fetched that collection via an apollo query", when we try to readQuery() we get an exception & app crash.

This feels like a good candidate to be supported in v3.0. It will break the interface of those who rely on the exception to catch this error, which can be justified in a major release.

[heymanhn]

@mcdougal thanks for that monkey-patching solution you suggested. Seems most prudent and the right place to catch the errors or invariant violations.

+1 on having the Apollo team address this in v3.0.