feat(graphql): query verifies the response (#767)

* feat(graphql): query verifies the response

* Fixed tests for API base

* More tests

* Refactor

* Updated tests

* Clearer GraphqlResponse model

* Fixed the model

* Fixed tests

Author: 2ndalpha

packages/example-graphql-events/app.ts

@@ -1,4 +1,9 @@
 import { Qminder } from 'qminder-api';
+import gql from 'graphql-tag';
+
+interface LocationsResponse {
+  locations: Location[];
+}
 
 interface Location {
   id: string;
@@ -16,25 +21,26 @@ interface TicketCreatedEvent {
 }
 
 async function findFirstLocationId(): Promise<Location> {
-  const result: any = await Qminder.GraphQL.query(`{
-      locations {
+  let result: LocationsResponse;
+  try {
+    result = await Qminder.GraphQL.query(gql`
+      {
+        locations {
           id
           name
+        }
       }
-  }`);
-
-  if (result.errors) {
-    throw new Error(
-      `Failed to find locations. Errors: ${JSON.stringify(result.errors)}`,
-    );
+    `);
+  } catch (e) {
+    throw new Error(`Failed to find locations. Error: ${JSON.stringify(e)}`);
   }
 
-  if (result.data.locations.length < 1) {
+  if (result.locations.length < 1) {
     throw new Error('Account does not have any locations');
   }
 
-  console.log(`Found ${result.data.locations.length} locations`);
-  return result.data.locations[0];
+  console.log(`Found ${result.locations.length} locations`);
+  return result.locations[0];
 }
 
 async function listenForTickets() {

packages/example-graphql-events/package.json

@@ -13,5 +13,8 @@
     "typescript": "5.7.2"
   },
   "author": "Qminder <support@qminder.com> (https://www.qminder.com)",
-  "license": "Apache-2.0"
+  "license": "Apache-2.0",
+  "dependencies": {
+    "graphql-tag": "^2.12.6"
+  }
 }

packages/javascript-api/src/lib/model/graphql-response.ts

@@ -1,11 +1,14 @@
-export interface GraphqlResponse {
-  /** An array that contains any GraphQL errors. */
-  errors?: GraphqlError[];
-  /** If the data was loaded without any errors, contains the requested object. */
-  data?: object;
+export type GraphqlResponse<T> = SuccessResponse<T> | ErrorResponse;
+
+export interface SuccessResponse<T> {
+  data: T;
+}
+
+export interface ErrorResponse {
+  errors: GraphqlError[];
 }
 
-interface GraphqlError {
+export interface GraphqlError {
   message: string;
   errorType: string;
   validationErrorType?: string;
@@ -14,3 +17,15 @@ interface GraphqlError {
   extensions?: any;
   locations: { line: number; column: number; sourceName: string }[];
 }
+
+export function isErrorResponse<T>(
+  response: GraphqlResponse<T>,
+): response is ErrorResponse {
+  return Object.prototype.hasOwnProperty.call(response, 'errors');
+}
+
+export function isSuccessResponse<T>(
+  response: GraphqlResponse<T>,
+): response is SuccessResponse<T> {
+  return Object.prototype.hasOwnProperty.call(response, 'data');
+}

packages/javascript-api/src/lib/services/api-base/api-base.spec.ts

@@ -7,6 +7,7 @@ import { ComplexError } from '../../model/errors/complex-error';
 import { SimpleError } from '../../model/errors/simple-error';
 import { UnknownError } from '../../model/errors/unknown-error';
 import { Qminder } from '../../qminder';
+import { ResponseValidationError } from '../../model/errors/response-validation-error';
 
 /**
  * A function that generates an object with the following keys:
@@ -37,8 +38,6 @@ function generateRequestData(query: string, responseData: any): any {
       body: JSON.stringify(queryObject),
     },
     successfulResponse: {
-      statusCode: 200,
-      errors: [],
       data: [responseData],
     },
   };
@@ -47,7 +46,7 @@ function generateRequestData(query: string, responseData: any): any {
 const FAKE_RESPONSE = {
   ok: true,
   json() {
-    return { statusCode: 200 };
+    return {};
   },
 };
 
@@ -436,7 +435,7 @@ describe('ApiBase', () => {
 
     it('throws when no query is passed', () => {
       Qminder.ApiBase.setKey('testing');
-      expect(() => (Qminder.ApiBase.queryGraph as any)()).toThrow();
+      expect(() => (Qminder.ApiBase.queryGraph as any)()).rejects.toThrow();
     });
 
     it('does not throw when no variables are passed', async () => {
@@ -452,7 +451,7 @@ describe('ApiBase', () => {
 
     it('throws when API key is not defined', () => {
       fetchSpy.mockReturnValue(new MockResponse(ME_ID.successfulResponse));
-      expect(() => Qminder.ApiBase.queryGraph(ME_ID.request)).toThrow();
+      expect(() => Qminder.ApiBase.queryGraph(ME_ID.request)).rejects.toThrow();
     });
 
     it('sends a correct request', () => {
@@ -464,42 +463,43 @@ describe('ApiBase', () => {
       expect(fetchSpy).toHaveBeenCalledWith(API_URL, ME_ID.expectedFetch);
     });
 
-    it('resolves with the entire response object, not only response data', (done) => {
+    it('resolves with response data', (done) => {
       Qminder.ApiBase.setKey('testing');
       fetchSpy.mockImplementation(() =>
         Promise.resolve(new MockResponse(ME_ID.successfulResponse)),
       );
 
       Qminder.ApiBase.queryGraph(ME_ID.request).then((response) => {
-        expect(response).toEqual(ME_ID.successfulResponse);
+        expect(response).toEqual(ME_ID.successfulResponse.data);
         done();
       });
     });
 
-    it('throws an error when getting errors as response', (done) => {
+    it('throws an error when getting errors as response', async () => {
       Qminder.ApiBase.setKey('testing');
       fetchSpy.mockImplementation(() =>
         Promise.resolve(new MockResponse(ERROR_UNDEFINED_FIELD)),
       );
 
-      Qminder.ApiBase.queryGraph(ME_ID.request).then(
-        () => done(new Error('QueryGraph should have thrown an error')),
-        () => done(),
+      expect(async () => {
+        await Qminder.ApiBase.queryGraph(ME_ID.request);
+      }).rejects.toThrow(
+        new SimpleError(
+          "Validation error of type FieldUndefined: Field 'x' in type 'Account' is undefined @ 'account/x'",
+        ),
       );
     });
 
-    it('should resolve with response, even if response has errors', (done) => {
+    it('should throw an error when response does not contain any data', async () => {
       Qminder.ApiBase.setKey('testing');
-      fetchSpy.mockImplementation(() =>
-        Promise.resolve(new MockResponse(ERROR_UNDEFINED_FIELD)),
-      );
-
-      Qminder.ApiBase.queryGraph(ME_ID.request).then(
-        () => done(new Error('Should have errored')),
-        (error: SimpleError) => {
-          expect(error.message).toEqual(VALIDATION_ERROR);
-          done();
-        },
+      fetchSpy.mockImplementation(() => Promise.resolve(FAKE_RESPONSE));
+
+      expect(async () => {
+        await Qminder.ApiBase.queryGraph(ME_ID.request);
+      }).rejects.toThrow(
+        new ResponseValidationError(
+          `Server response is not valid GraphQL response. Response: {}`,
+        ),
       );
     });
   });

packages/javascript-api/src/lib/services/api-base/api-base.ts

@@ -1,9 +1,14 @@
-import { GraphQLError } from 'graphql';
 import { ComplexError } from '../../model/errors/complex-error.js';
 import { SimpleError } from '../../model/errors/simple-error.js';
 import { UnknownError } from '../../model/errors/unknown-error.js';
-import { GraphqlResponse } from '../../model/graphql-response.js';
+import {
+  ErrorResponse,
+  GraphqlResponse,
+  isErrorResponse,
+  isSuccessResponse,
+} from '../../model/graphql-response.js';
 import { RequestInit } from '../../model/fetch.js';
+import { ResponseValidationError } from '../../model/errors/response-validation-error.js';
 
 type RequestInitWithMethodRequired = Pick<RequestInit, 'method' | 'headers'> & {
   body?: string | File | object;
@@ -121,7 +126,7 @@ export class ApiBase {
    * @throws when the API key is missing or invalid, or when errors in the
    * response are found
    */
-  static queryGraph(query: GraphqlQuery): Promise<GraphqlResponse> {
+  static async queryGraph<T>(query: GraphqlQuery): Promise<T> {
     if (!this.apiKey) {
       throw new Error('Please set the API key before making any requests.');
     }
@@ -136,17 +141,21 @@ export class ApiBase {
       body: JSON.stringify(query),
     };
 
-    return fetch(`https://${this.apiServer}/graphql`, init)
-      .then((response: Response) => response.json())
-      .then((responseJson: any) => {
-        if (responseJson.errorMessage) {
-          throw new Error(responseJson.errorMessage);
-        }
-        if (responseJson.errors && responseJson.errors.length > 0) {
-          throw this.extractGraphQLError(responseJson);
-        }
-        return responseJson as Promise<GraphqlResponse>;
-      });
+    let response = await fetch(`https://${this.apiServer}/graphql`, init);
+    let graphQLResponse: GraphqlResponse<T> = await response.json();
+
+    if (isErrorResponse(graphQLResponse)) {
+      throw this.extractGraphQLError(graphQLResponse);
+    }
+    if (isSuccessResponse(graphQLResponse)) {
+      return graphQLResponse.data;
+    }
+
+    throw new ResponseValidationError(
+      `Server response is not valid GraphQL response. Response: ${JSON.stringify(
+        graphQLResponse,
+      )}`,
+    );
   }
 
   private static extractError(response: any): Error {
@@ -169,9 +178,7 @@ export class ApiBase {
     return new UnknownError();
   }
 
-  private static extractGraphQLError(response: {
-    errors: GraphQLError[];
-  }): Error {
+  private static extractGraphQLError(response: ErrorResponse): Error {
     return new SimpleError(
       response.errors.map((error) => error.message).join('\n'),
     );

packages/javascript-api/src/lib/services/graphql/__tests__/graphql.service.spec.ts

@@ -8,7 +8,14 @@ import { GraphqlService } from '../graphql.service';
 jest.mock('isomorphic-ws', () => jest.fn());
 
 describe('GraphQL service', function () {
-  const ME_ID_REQUEST = '{ me { id } }';
+  const ME_ID_REQUEST = gql`
+    {
+      me {
+        id
+      }
+    }
+  `;
+  const ME_ID_REQUEST_STRING = '{ me { id }\n}';
   const ME_ID_SUCCESS_RESPONSE: any = {
     statusCode: 200,
     data: [
@@ -45,25 +52,25 @@ describe('GraphQL service', function () {
     });
     it('calls ApiBase.queryGraph with the correct parameters', async () => {
       await graphqlService.query(ME_ID_REQUEST);
-      const graphqlQuery = { query: ME_ID_REQUEST };
+      const graphqlQuery = { query: ME_ID_REQUEST_STRING };
       expect(requestStub.calledWith(graphqlQuery)).toBeTruthy();
     });
     it('calls ApiBase.queryGraph with both query & variables', async () => {
       const variables = { x: 5, y: 4 };
       await graphqlService.query(ME_ID_REQUEST, variables);
-      const graphqlQuery = { query: ME_ID_REQUEST, variables };
+      const graphqlQuery = { query: ME_ID_REQUEST_STRING, variables };
       expect(requestStub.calledWith(graphqlQuery)).toBeTruthy();
     });
     it('collapses whitespace and newlines', async () => {
-      const query = `
+      const query = gql`
         {
           me {
             id
           }
         }
       `;
       await graphqlService.query(query);
-      const graphqlQuery = { query: ME_ID_REQUEST };
+      const graphqlQuery = { query: ME_ID_REQUEST_STRING };
       expect(requestStub.calledWith(graphqlQuery)).toBeTruthy();
     });
 

packages/javascript-api/src/lib/services/graphql/graphql.service.ts

@@ -8,7 +8,6 @@ import WebSocket, { CloseEvent } from 'isomorphic-ws';
 import { Observable, Observer, startWith, Subject } from 'rxjs';
 import { distinctUntilChanged, shareReplay } from 'rxjs/operators';
 import { ConnectionStatus } from '../../model/connection-status.js';
-import { GraphqlResponse } from '../../model/graphql-response.js';
 import { calculateRandomizedExponentialBackoffTime } from '../../util/randomized-exponential-backoff/randomized-exponential-backoff.js';
 import { sleepMs } from '../../util/sleep-ms/sleep-ms.js';
 import { ApiBase, GraphqlQuery } from '../api-base/api-base.js';
@@ -146,22 +145,16 @@ export class GraphqlService {
    * });
    * ```
    *
-   * @param query required: the query to send, for example `"{ me { selectedLocation } }"`
+   * @param queryDocument required: the query to send, for example gql`{ me { selectedLocation } }`
    * @param variables optional: additional variables for the query, if variables were used
    * @returns a promise that resolves to the query's results, or rejects if the query failed
    * @throws when the 'query' argument is undefined or an empty string
    */
-  query(
-    queryDocument: QueryOrDocument,
+  query<T>(
+    queryDocument: DocumentNode,
     variables?: { [key: string]: any },
-  ): Promise<GraphqlResponse> {
-    const query = queryToString(queryDocument);
-    if (!query || query.length === 0) {
-      throw new Error(
-        'GraphQLService query expects a GraphQL query as its first argument',
-      );
-    }
-
+  ): Promise<T> {
+    const query = print(queryDocument);
     const packedQuery = query.replace(/\s\s+/g, ' ').trim();
     const graphqlQuery: GraphqlQuery = {
       query: packedQuery,

yarn.lock

@@ -4562,7 +4562,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"graphql-tag@npm:2.12.6":
+"graphql-tag@npm:2.12.6, graphql-tag@npm:^2.12.6":
   version: 2.12.6
   resolution: "graphql-tag@npm:2.12.6"
   dependencies:
@@ -6767,6 +6767,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "qminder-graphql-events-example@workspace:packages/example-graphql-events"
   dependencies:
+    graphql-tag: "npm:^2.12.6"
     tsx: "npm:^4.19.2"
     typescript: "npm:5.7.2"
   languageName: unknown