Fix bug in Connections with errors

Summary:
This diff fixes a bug in the connection handler where errors that occur on a field are not copied into the "connection" record for that field. Connections create virtual fields on a record that contain the results for the set of edges fetched with a set of args. These are merged into the record by the connection handler. However, the errors are only ever set on the base field in the normalizer. By copying the fields over to the connection field, the `throwOnFieldError` directive now works correctly on connections.

This diff also adds some public APIs to the RecordProxy and mutator to deal with errors, which have not been handled in Relay previously.

Reviewed By: captbaritone

Differential Revision: D64505652

fbshipit-source-id: 288b33bf047d5baf7dacd0961ba6b5692766be0e

packages/relay-runtime/handlers/connection/ConnectionHandler.js

@@ -67,7 +67,12 @@ function update(store: RecordSourceProxy, payload: HandleFieldPayload): void {
   const serverPageInfo =
     serverConnection && serverConnection.getLinkedRecord(PAGE_INFO);
   if (!serverConnection) {
-    record.setValue(null, payload.handleKey);
+    record.setValue(
+      null,
+      payload.handleKey,
+      undefined,
+      record.getErrors(payload.fieldKey),
+    );
     return;
   }
   // In rare cases the handleKey field may be unset even though the client

packages/relay-runtime/handlers/connection/__tests__/ConnectionHandler-test.js

@@ -45,7 +45,7 @@ describe('ConnectionHandler', () => {
   let proxy;
   let sinkSource;
 
-  function normalize(payload, variables, options) {
+  function normalize(payload, variables, options, errors) {
     RelayResponseNormalizer.normalize(
       baseSource,
       createNormalizationSelector(
@@ -57,6 +57,7 @@ describe('ConnectionHandler', () => {
       options ?? {
         getDataID: defaultGetDataID,
       },
+      errors,
     );
   }
 
@@ -132,6 +133,110 @@ describe('ConnectionHandler', () => {
     });
   });
 
+  describe('field errors', () => {
+    it('propagates errors to virtual connection field from server connection field', () => {
+      normalize(
+        {
+          node: {
+            id: '4',
+            __typename: 'User',
+            friends: null,
+          },
+        },
+        {
+          after: null,
+          before: null,
+          count: 10,
+          orderby: ['first name'],
+          id: '4',
+        },
+        undefined,
+        [{message: 'Oops!', path: ['node', 'friends']}],
+      );
+      const args = {first: 10, orderby: ['first name']};
+      const handleKey =
+        getRelayHandleKey('connection', 'ConnectionQuery_friends', 'friends') +
+        '(orderby:["first name"])';
+      const payload = {
+        args,
+        dataID: '4',
+        fieldKey: getStableStorageKey('friends', args),
+        handleKey,
+      };
+      ConnectionHandler.update(proxy, payload);
+      expect(sinkSource.toJSON()['4'].__errors).toEqual({
+        '__ConnectionQuery_friends_connection(orderby:["first name"])': [
+          {message: 'Oops!'},
+        ],
+      });
+    });
+
+    it('leaves errors when a valid value is added', () => {
+      normalize(
+        {
+          node: {
+            id: '4',
+            __typename: 'User',
+            friends: null,
+          },
+        },
+        {
+          after: null,
+          before: null,
+          count: 10,
+          orderby: ['first name'],
+          id: '4',
+        },
+        undefined,
+        [{message: 'Oops!', path: ['node', 'friends']}],
+      );
+
+      const args = {first: 10, orderby: ['first name']};
+      const handleKey =
+        getRelayHandleKey('connection', 'ConnectionQuery_friends', 'friends') +
+        '(orderby:["first name"])';
+      const payload = {
+        args,
+        dataID: '4',
+        fieldKey: getStableStorageKey('friends', args),
+        handleKey,
+      };
+      ConnectionHandler.update(proxy, payload);
+      // Re-check that an error is set (same as basic error test)
+      expect(sinkSource.toJSON()['4'].__errors).toEqual({
+        '__ConnectionQuery_friends_connection(orderby:["first name"])': [
+          {message: 'Oops!'},
+        ],
+      });
+
+      // Check that the error is not cleared even if new data arrives
+      normalize(
+        {
+          node: {
+            id: '4',
+            __typename: 'User',
+            friends: [],
+          },
+        },
+        {
+          after: null,
+          before: null,
+          count: 10,
+          orderby: ['first name'],
+          id: '4',
+        },
+        undefined,
+        undefined,
+      );
+      ConnectionHandler.update(proxy, payload);
+      expect(sinkSource.toJSON()['4'].__errors).toEqual({
+        '__ConnectionQuery_friends_connection(orderby:["first name"])': [
+          {message: 'Oops!'},
+        ],
+      });
+    });
+  });
+
   describe('insertEdgeAfter()', () => {
     let connection;
     let connectionID;

packages/relay-runtime/mutations/RelayRecordProxy.js

@@ -11,6 +11,7 @@
 
 'use strict';
 
+import type {TRelayFieldError} from '../store/RelayErrorTrie';
 import type {RecordProxy} from '../store/RelayStoreTypes';
 import type {Arguments} from '../store/RelayStoreUtils';
 import type {DataID} from '../util/RelayRuntimeTypes';
@@ -65,22 +66,48 @@ class RelayRecordProxy implements RecordProxy {
     return this._mutator.getValue(this._dataID, storageKey);
   }
 
-  setValue(value: mixed, name: string, args?: ?Arguments): RecordProxy {
+  setValue(
+    value: mixed,
+    name: string,
+    args?: ?Arguments,
+    errors?: ?$ReadOnlyArray<TRelayFieldError>,
+  ): RecordProxy {
     invariant(
       isValidLeafValue(value),
       'RelayRecordProxy#setValue(): Expected a scalar or array of scalars, ' +
         'got `%s`.',
       JSON.stringify(value),
     );
-    return this.setValue__UNSAFE(value, name, args);
+
+    return this.setValue__UNSAFE(value, name, args, errors);
+  }
+
+  getErrors(
+    name: string,
+    args?: ?Arguments,
+  ): ?$ReadOnlyArray<TRelayFieldError> {
+    const storageKey = getStableStorageKey(name, args);
+    return this._mutator.getErrors(this._dataID, storageKey);
   }
 
   // This is used in the typesafe updaters.
   // We already validated that the value has the correct type
   // so it should be safe to store complex structures as scalar values (custom scalars)
-  setValue__UNSAFE(value: mixed, name: string, args?: ?Arguments): RecordProxy {
+  setValue__UNSAFE(
+    value: mixed,
+    name: string,
+    args?: ?Arguments,
+    errors?: ?$ReadOnlyArray<TRelayFieldError>,
+  ): RecordProxy {
     const storageKey = getStableStorageKey(name, args);
     this._mutator.setValue(this._dataID, storageKey, value);
+    if (errors != null) {
+      if (errors.length === 0) {
+        this._mutator.setErrors(this._dataID, storageKey);
+      } else {
+        this._mutator.setErrors(this._dataID, storageKey, errors);
+      }
+    }
     return this;
   }
 

packages/relay-runtime/mutations/RelayRecordSourceMutator.js

@@ -11,6 +11,7 @@
 
 'use strict';
 
+import type {TRelayFieldError} from '../store/RelayErrorTrie';
 import type {RecordState} from '../store/RelayRecordState';
 import type {
   MutableRecordSource,
@@ -179,6 +180,32 @@ class RelayRecordSourceMutator {
     RelayModernRecord.setValue(sinkRecord, storageKey, value);
   }
 
+  getErrors(
+    dataID: DataID,
+    storageKey: string,
+  ): ?$ReadOnlyArray<TRelayFieldError> {
+    for (let ii = 0; ii < this.__sources.length; ii++) {
+      const record = this.__sources[ii].get(dataID);
+      if (record) {
+        const value = RelayModernRecord.getErrors(record, storageKey);
+        if (value !== undefined) {
+          return value;
+        }
+      } else if (record === null) {
+        return null;
+      }
+    }
+  }
+
+  setErrors(
+    dataID: DataID,
+    storageKey: string,
+    errors?: $ReadOnlyArray<TRelayFieldError>,
+  ): void {
+    const sinkRecord = this._getSinkRecord(dataID);
+    RelayModernRecord.setErrors(sinkRecord, storageKey, errors);
+  }
+
   getLinkedRecordID(dataID: DataID, storageKey: string): ?DataID {
     for (let ii = 0; ii < this.__sources.length; ii++) {
       const record = this.__sources[ii].get(dataID);

packages/relay-runtime/store/RelayModernRecord.js

@@ -35,7 +35,7 @@ const areEqual = require('areEqual');
 const invariant = require('invariant');
 const warning = require('warning');
 
-type StorageKey = Exclude<string, typeof ERRORS_KEY>;
+export type StorageKey = Exclude<string, typeof ERRORS_KEY>;
 
 type RelayFieldErrors = {[StorageKey]: $ReadOnlyArray<TRelayFieldError>};
 

packages/relay-runtime/store/RelayStoreTypes.js

@@ -467,6 +467,7 @@ export interface RecordProxy {
   ): RecordProxy;
   getType(): string;
   getValue(name: string, args?: ?Variables): mixed;
+  getErrors(name: string, args?: ?Variables): ?$ReadOnlyArray<TRelayFieldError>;
   setLinkedRecord(
     record: RecordProxy,
     name: string,
@@ -477,7 +478,12 @@ export interface RecordProxy {
     name: string,
     args?: ?Variables,
   ): RecordProxy;
-  setValue(value: mixed, name: string, args?: ?Variables): RecordProxy;
+  setValue(
+    value: mixed,
+    name: string,
+    args?: ?Variables,
+    errors?: ?$ReadOnlyArray<TRelayFieldError>,
+  ): RecordProxy;
   invalidateRecord(): void;
 }