Better JSON messages (#8379)

Author: maneesht

packages/data-connect/CHANGELOG.md

@@ -2,4 +2,4 @@
 ## UNRELEASED
 * Updated reporting to use @firebase/data-connect instead of @firebase/connect
 * Added functionality to retry queries and mutations if the server responds with UNAUTHENTICATED.
-
+* Updated errors to only show relevant details to the user.

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

@@ -64,17 +64,15 @@ export function dcFetch<T, U>(
       } catch (e) {
         throw new DataConnectError(Code.OTHER, JSON.stringify(e));
       }
+      const message = getMessage(jsonResponse);
       if (response.status >= 400) {
         logError(
           'Error while performing request: ' + JSON.stringify(jsonResponse)
         );
         if (response.status === 401) {
-          throw new DataConnectError(
-            Code.UNAUTHORIZED,
-            JSON.stringify(jsonResponse)
-          );
+          throw new DataConnectError(Code.UNAUTHORIZED, message);
         }
-        throw new DataConnectError(Code.OTHER, JSON.stringify(jsonResponse));
+        throw new DataConnectError(Code.OTHER, message);
       }
       return jsonResponse;
     })
@@ -87,3 +85,12 @@ export function dcFetch<T, U>(
       return res as { data: T; errors: Error[] };
     });
 }
+interface MessageObject {
+  message?: string;
+}
+function getMessage(obj: MessageObject): string {
+  if ('message' in obj) {
+    return obj.message;
+  }
+  return JSON.stringify(obj);
+}

packages/data-connect/test/emulatorSeeder.ts

@@ -78,7 +78,7 @@ export async function setupQueries(
         })
       }
     },
-    // eslint-disable-line camelcase
+    // eslint-disable-next-line camelcase
     connection_string:
       'postgresql://postgres:secretpassword@localhost:5432/postgres?sslmode=disable'
   };

packages/data-connect/test/post.gql

@@ -9,4 +9,10 @@ query listPosts @auth(level: PUBLIC) {
     content
   }
 }
+query listPosts2  {
+  posts {
+    id,
+    content
+  }
+}
 

packages/data-connect/test/queries.test.ts

@@ -59,6 +59,10 @@ const SEEDED_DATA = [
     content: 'task 2'
   }
 ];
+const REAL_DATA = SEEDED_DATA.map(obj => ({
+  ...obj,
+  id: obj.id.replace(/-/g, '')
+}));
 function seedDatabase(instance: DataConnect): Promise<void> {
   // call mutation query that adds SEEDED_DATA to database
   return new Promise((resolve, reject) => {
@@ -100,7 +104,7 @@ describe('DataConnect Tests', async () => {
     const taskListQuery = queryRef<TaskListResponse>(dc, 'listPosts');
     const taskListRes = await executeQuery(taskListQuery);
     expect(taskListRes.data).to.deep.eq({
-      posts: SEEDED_DATA
+      posts: REAL_DATA
     });
   });
   it(`instantly executes a query if one hasn't been subscribed to`, async () => {
@@ -121,7 +125,7 @@ describe('DataConnect Tests', async () => {
     );
     const res = await promise;
     expect(res.data).to.deep.eq({
-      posts: SEEDED_DATA
+      posts: REAL_DATA
     });
     expect(res.source).to.eq(SOURCE_SERVER);
   });
@@ -138,7 +142,7 @@ describe('DataConnect Tests', async () => {
     const result = await waitForFirstEvent(taskListQuery);
     const serializedRef: SerializedRef<TaskListResponse, undefined> = {
       data: {
-        posts: SEEDED_DATA
+        posts: REAL_DATA
       },
       fetchTime: Date.now().toLocaleString(),
       refInfo: {
@@ -149,7 +153,7 @@ describe('DataConnect Tests', async () => {
         name: taskListQuery.name,
         variables: undefined
       },
-      source: SOURCE_SERVER
+      source: SOURCE_CACHE
     };
     expect(result.toJSON()).to.deep.eq(serializedRef);
     expect(result.source).to.deep.eq(SOURCE_CACHE);
@@ -167,6 +171,14 @@ describe('DataConnect Tests', async () => {
       'ECONNREFUSED'
     );
   });
+  it('throws an error with just the message when the server responds with an error', async () => {
+    const invalidTaskListQuery = queryRef<TaskListResponse>(dc, 'listPosts2');
+    const message =
+      'unauthorized: you are not authorized to perform this operation';
+    await expect(
+      executeQuery(invalidTaskListQuery)
+    ).to.eventually.be.rejectedWith(message);
+  });
 });
 async function waitForFirstEvent<Data, Variables>(
   query: QueryRef<Data, Variables>

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

@@ -22,9 +22,9 @@ import { ConnectorConfig, getDataConnect } from '../../src';
 
 describe('Data Connect Test', () => {
   it('should throw an error if `projectId` is not provided', async () => {
-    const app = initializeApp({});
+    const app = initializeApp({ projectId: undefined }, 'a');
     expect(() =>
-      getDataConnect({ connector: 'c', location: 'l', service: 's' })
+      getDataConnect(app, { connector: 'c', location: 'l', service: 's' })
     ).to.throw(
       'Project ID must be provided. Did you pass in a proper projectId to initializeApp?'
     );

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

@@ -0,0 +1,57 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect, use } from 'chai';
+import chaiAsPromised from 'chai-as-promised';
+import * as sinon from 'sinon';
+
+import { dcFetch, initializeFetch } from '../../src/network/fetch';
+use(chaiAsPromised);
+function mockFetch(json: object): void {
+  const fakeFetchImpl = sinon.stub().returns(
+    Promise.resolve({
+      json: () => {
+        return Promise.resolve(json);
+      },
+      status: 401
+    } as Response)
+  );
+  initializeFetch(fakeFetchImpl);
+}
+describe('fetch', () => {
+  it('should throw an error with just the message when the server responds with an error with a message property in the body', async () => {
+    const message = 'Failed to connect to Postgres instance';
+    mockFetch({
+      code: 401,
+      message
+    });
+    await expect(
+      dcFetch('http://localhost', {}, {} as AbortController, null)
+    ).to.eventually.be.rejectedWith(message);
+  });
+  it('should throw a stringified message when the server responds with an error without a message property in the body', async () => {
+    const message = 'Failed to connect to Postgres instance';
+    const json = {
+      code: 401,
+      message1: message
+    };
+    mockFetch(json);
+    await expect(
+      dcFetch('http://localhost', {}, {} as AbortController, null)
+    ).to.eventually.be.rejectedWith(JSON.stringify(json));
+  });
+});

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

@@ -70,7 +70,7 @@ describe('Queries', () => {
     const authProvider = new FakeAuthProvider();
     const rt = new RESTTransport(options, undefined, authProvider);
     await expect(rt.invokeQuery('test', null)).to.eventually.be.rejectedWith(
-      JSON.stringify(json)
+      json.message
     );
     expect(fakeFetchImpl.callCount).to.eq(2);
   });
@@ -79,7 +79,7 @@ describe('Queries', () => {
     const authProvider = new FakeAuthProvider();
     const rt = new RESTTransport(options, undefined, authProvider);
     await expect(rt.invokeMutation('test', null)).to.eventually.be.rejectedWith(
-      JSON.stringify(json)
+      json.message
     );
     expect(fakeFetchImpl.callCount).to.eq(2);
   });
@@ -90,7 +90,7 @@ describe('Queries', () => {
     rt._setLastToken('initial token');
     await expect(
       rt.invokeQuery('test', null) as Promise<unknown>
-    ).to.eventually.be.rejectedWith(JSON.stringify(json));
+    ).to.eventually.be.rejectedWith(json.message);
     expect(fakeFetchImpl.callCount).to.eq(1);
   });
 });