Implement Partial Errors for FDC

.changeset/sharp-nails-glow.md

@@ -0,0 +1,5 @@
+---
+"@firebase/data-connect": patch
+---
+
+Expose partial errors to the user.

common/api-review/data-connect.api.md

@@ -52,6 +52,35 @@ export class DataConnect {
     setInitialized(): void;
 }
 
+// @public
+export class DataConnectError extends FirebaseError {
+    }
+
+// @public (undocumented)
+export type DataConnectErrorCode = 'other' | 'already-initialized' | 'not-initialized' | 'not-supported' | 'invalid-argument' | 'partial-error' | 'unauthorized';
+
+// @public
+export class DataConnectOperationError extends DataConnectError {
+    /* Excluded from this release type: name */
+    readonly response: DataConnectOperationFailureResponse;
+}
+
+// @public (undocumented)
+export interface DataConnectOperationFailureResponse {
+    // (undocumented)
+    readonly data?: Record<string, unknown> | null;
+    // (undocumented)
+    readonly errors: DataConnectOperationFailureResponseErrorInfo[];
+}
+
+// @public (undocumented)
+export interface DataConnectOperationFailureResponseErrorInfo {
+    // (undocumented)
+    readonly message: string;
+    // (undocumented)
+    readonly path: Array<string | number>;
+}
+
 // @public
 export interface DataConnectOptions extends ConnectorConfig {
     // (undocumented)
@@ -67,7 +96,7 @@ export interface DataConnectResult<Data, Variables> extends OpResult<Data> {
 // @public
 export interface DataConnectSubscription<Data, Variables> {
     // (undocumented)
-    errCallback?: (e?: FirebaseError) => void;
+    errCallback?: (e?: DataConnectError) => void;
     // (undocumented)
     unsubscribe: () => void;
     // (undocumented)
@@ -118,7 +147,7 @@ export interface MutationResult<Data, Variables> extends DataConnectResult<Data,
 export type OnCompleteSubscription = () => void;
 
 // @public
-export type OnErrorSubscription = (err?: FirebaseError) => void;
+export type OnErrorSubscription = (err?: DataConnectError) => void;
 
 // @public
 export type OnResultSubscription<Data, Variables> = (res: QueryResult<Data, Variables>) => void;

packages/data-connect/src/api/index.ts

@@ -22,3 +22,10 @@ export * from './Mutation';
 export * from './query';
 export { setLogLevel } from '../logger';
 export { validateArgs } from '../util/validateArgs';
+export {
+  DataConnectErrorCode,
+  DataConnectError,
+  DataConnectOperationError,
+  DataConnectOperationFailureResponse,
+  DataConnectOperationFailureResponseErrorInfo
+} from '../core/error';

packages/data-connect/src/core/error.ts

@@ -40,25 +40,62 @@ export const Code = {
 
 /** An error returned by a DataConnect operation. */
 export class DataConnectError extends FirebaseError {
-  /** The stack of the error. */
-  readonly stack?: string;
+  /** @internal */
+  readonly name: string = 'DataConnectError';
 
   /** @hideconstructor */
-  constructor(
-    /**
-     * The backend error code associated with this error.
-     */
-    readonly code: DataConnectErrorCode,
-    /**
-     * A custom error description.
-     */
-    readonly message: string
-  ) {
+  constructor(code: Code, message: string) {
     super(code, message);
 
-    // HACK: We write a toString property directly because Error is not a real
-    // class and so inheritance does not work correctly. We could alternatively
-    // do the same "back-door inheritance" trick that FirebaseError does.
-    this.toString = () => `${this.name}: [code=${this.code}]: ${this.message}`;
+    // Ensure the instanceof operator works as expected on subclasses of Error.
+    // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#custom_error_types
+    // and https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget
+    Object.setPrototypeOf(this, DataConnectError.prototype);
   }
+
+  /** @internal */
+  toString(): string {
+    return `${this.name}[code=${this.code}]: ${this.message}`;
+  }
+}
+
+/** An error returned by a DataConnect operation. */
+export class DataConnectOperationError extends DataConnectError {
+  /** @internal */
+  readonly name: string = 'DataConnectOperationError';
+
+  /** The response received from the backend. */
+  readonly response: DataConnectOperationFailureResponse;
+
+  /** @hideconstructor */
+  constructor(message: string, response: DataConnectOperationFailureResponse) {
+    super(Code.PARTIAL_ERROR, message);
+    this.response = response;
+  }
+}
+
+export interface DataConnectOperationFailureResponse {
+  // The "data" provided by the backend in the response message.
+  //
+  // Will be `undefined` if no "data" was provided in the response message.
+  // Otherwise, will be `null` if `null` was explicitly specified as the "data"
+  // in the response message. Otherwise, will be the value of the "data"
+  // specified as the "data" in the response message
+  readonly data?: Record<string, unknown> | null;
+
+  // The list of errors provided by the backend in the response message.
+  readonly errors: DataConnectOperationFailureResponseErrorInfo[];
+}
+
+// Information about the error, as provided in the response from the backend.
+// See https://spec.graphql.org/draft/#sec-Errors
+export interface DataConnectOperationFailureResponseErrorInfo {
+  // The error message.
+  readonly message: string;
+
+  // The path of the field in the response data to which this error relates.
+  // String values in this array refer to field names. Numeric values in this
+  // array always satisfy `Number.isInteger()` and refer to the index in an
+  // array.
+  readonly path: Array<string | number>;
 }

packages/data-connect/src/network/fetch.ts

@@ -15,7 +15,12 @@
  * limitations under the License.
  */
 
-import { Code, DataConnectError } from '../core/error';
+import {
+  Code,
+  DataConnectError,
+  DataConnectOperationError,
+  DataConnectOperationFailureResponse
+} from '../core/error';
 import { SDK_VERSION } from '../core/version';
 import { logDebug, logError } from '../logger';
 
@@ -108,8 +113,14 @@ export function dcFetch<T, U>(
     .then(res => {
       if (res.errors && res.errors.length) {
         const stringified = JSON.stringify(res.errors);
-        logError('DataConnect error while performing request: ' + stringified);
-        throw new DataConnectError(Code.OTHER, stringified);
+        const response: DataConnectOperationFailureResponse = {
+          errors: res.errors,
+          data: res.data
+        };
+        throw new DataConnectOperationError(
+          'DataConnect error while performing request: ' + stringified,
+          response
+        );
       }
       return res;
     });

packages/data-connect/test/dataconnect/connector/connector.yaml

@@ -1,6 +1,6 @@
 connectorId: "tests"
 authMode: "PUBLIC" 
 generate:
-    javascriptSdk:
-      outputDir: "./gen/web"
-      package: "@test-app/tests"
+  javascriptSdk:
+    outputDir: "./gen/web"
+    package: "@test-app/tests"

packages/data-connect/test/unit/fetch.test.ts

@@ -85,6 +85,40 @@ describe('fetch', () => {
       )
     ).to.eventually.be.rejectedWith(JSON.stringify(json));
   });
+  it('should throw a stringified message when the server responds with an error without a message property in the body', async () => {
+    const json = {
+      'data': { 'abc': 'def' },
+      'errors': [
+        {
+          'message':
+            'SQL query error: pq: duplicate key value violates unique constraint movie_pkey',
+          'locations': [],
+          'path': ['the_matrix'],
+          'extensions': null
+        }
+      ]
+    };
+    mockFetch(json, false);
+    await expect(
+      dcFetch(
+        'http://localhost',
+        {
+          name: 'n',
+          operationName: 'n',
+          variables: {}
+        },
+        {} as AbortController,
+        null,
+        null,
+        null,
+        false,
+        CallerSdkTypeEnum.Base
+      )
+    ).to.eventually.be.rejected.then(error => {
+      expect(error.response.data).to.eq(json.data);
+      expect(error.response.errors).to.eq(json.errors);
+    });
+  });
   it('should assign different values to custom headers based on the _callerSdkType argument (_isUsingGen is false)', async () => {
     const json = {
       code: 200,

scripts/emulator-testing/emulators/dataconnect-emulator.ts

@@ -18,7 +18,7 @@
 import { platform } from 'os';
 import { Emulator } from './emulator';
 
-const DATACONNECT_EMULATOR_VERSION = '1.7.5';
+const DATACONNECT_EMULATOR_VERSION = '1.9.2';
 
 export class DataConnectEmulator extends Emulator {
   constructor(port = 9399) {

scripts/emulator-testing/emulators/emulator.ts

@@ -146,6 +146,7 @@ export abstract class Emulator {
       if (this.isDataConnect) {
         const dataConnectConfigDir = this.findDataConnectConfigDir();
         promise = spawn(this.binaryPath, [
+          '--logtostderr',
           '--v=2',
           'dev',
           `--listen=127.0.0.1:${this.port},[::1]:${this.port}`,
@@ -155,6 +156,9 @@ export abstract class Emulator {
         promise.childProcess.stderr?.on('data', res =>
           console.log(res.toString())
         );
+        promise.childProcess.stderr?.on('error', res =>
+          console.log(res.toString())
+        );
       } else {
         promise = spawn(
           'java',
@@ -174,7 +178,6 @@ export abstract class Emulator {
       promise.catch(reject);
       this.emulator = promise.childProcess;
 
-      console.log(`Waiting for emulator to start up ...`);
       // NOTE: Normally the emulator starts up within a few seconds.
       // However, our sdk test suite launches tests from 20+ packages in parallel, which slows
       // down the startup substantially. In such case for the emulator to start, it can take