Return graphql errors when exists (twentyhq#5389)

- throw badRequest with graphql error messages when graphql request fails - clean some code Before <img width="1470" alt="image" src="https://github.com/twentyhq/twenty/assets/29927851/0b700d9a-2bbe-41f7-84a9-981dc7dd5344"> After ![image](https://github.com/twentyhq/twenty/assets/29927851/6bbaaf7c-1244-473d-9ae5-4fefc6a1b994)
synapsenet-arena · May 14, 2024 · ffdd3a7 · ffdd3a7
1 parent 1bc9b78
commit ffdd3a7
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 81 deletions.
diff --git a/...ty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.service.ts b/...ty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.service.ts
@@ -483,6 +483,7 @@ export class WorkspaceQueryRunnerService {
     const { workspaceId, userId, objectMetadataItem } = options;
 
     assertMutationNotOnRemoteObject(objectMetadataItem);
+    assertIsValidUuid(args.id);
 
     const query = await this.workspaceQueryBuilderFactory.deleteOne(
       args,

diff --git a/...wenty-server/src/engine/api/rest/api-rest-query-builder/api-rest-query-builder.factory.ts b/...wenty-server/src/engine/api/rest/api-rest-query-builder/api-rest-query-builder.factory.ts
@@ -98,7 +98,7 @@ export class ApiRestQueryBuilderFactory {
     };
   }
 
-  async create(request): Promise<ApiRestQuery> {
+  async create(request: Request): Promise<ApiRestQuery> {
     const objectMetadata = await this.getObjectMetadata(request);
 
     const depth = computeDepth(request);
@@ -109,7 +109,7 @@ export class ApiRestQueryBuilderFactory {
     };
   }
 
-  async update(request): Promise<ApiRestQuery> {
+  async update(request: Request): Promise<ApiRestQuery> {
     const objectMetadata = await this.getObjectMetadata(request);
 
     const depth = computeDepth(request);
@@ -128,7 +128,7 @@ export class ApiRestQueryBuilderFactory {
     };
   }
 
-  async get(request): Promise<ApiRestQuery> {
+  async get(request: Request): Promise<ApiRestQuery> {
     const objectMetadata = await this.getObjectMetadata(request);
 
     const depth = computeDepth(request);

diff --git a/...y-server/src/engine/api/rest/api-rest-query-builder/factories/create-variables.factory.ts b/...y-server/src/engine/api/rest/api-rest-query-builder/factories/create-variables.factory.ts
@@ -1,5 +1,7 @@
 import { Injectable } from '@nestjs/common';
 
+import { Request } from 'express';
+
 import { ApiRestQueryVariables } from 'src/engine/api/rest/types/api-rest-query-variables.type';
 
 @Injectable()

diff --git a/...y-server/src/engine/api/rest/api-rest-query-builder/factories/update-variables.factory.ts b/...y-server/src/engine/api/rest/api-rest-query-builder/factories/update-variables.factory.ts
@@ -1,5 +1,7 @@
 import { Injectable } from '@nestjs/common';
 
+import { Request } from 'express';
+
 import { ApiRestQueryVariables } from 'src/engine/api/rest/types/api-rest-query-variables.type';
 
 @Injectable()

diff --git a/packages/twenty-server/src/engine/api/rest/api-rest.controller.ts b/packages/twenty-server/src/engine/api/rest/api-rest.controller.ts
@@ -3,29 +3,37 @@ import { Controller, Delete, Get, Post, Put, Req, Res } from '@nestjs/common';
 import { Request, Response } from 'express';
 
 import { ApiRestService } from 'src/engine/api/rest/api-rest.service';
-import { handleResult } from 'src/engine/api/rest/api-rest.controller.utils';
+import { cleanGraphQLResponse } from 'src/engine/api/rest/api-rest.controller.utils';
 
 @Controller('rest/*')
 export class ApiRestController {
   constructor(private readonly apiRestService: ApiRestService) {}
 
   @Get()
   async handleApiGet(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.get(request));
+    const result = await this.apiRestService.get(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 
   @Delete()
   async handleApiDelete(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.delete(request));
+    const result = await this.apiRestService.delete(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 
   @Post()
   async handleApiPost(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.create(request));
+    const result = await this.apiRestService.create(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 
   @Put()
   async handleApiPut(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.update(request));
+    const result = await this.apiRestService.update(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 }
diff --git a/packages/twenty-server/src/engine/api/rest/api-rest.controller.utils.ts b/packages/twenty-server/src/engine/api/rest/api-rest.controller.utils.ts
@@ -1,12 +1,8 @@
-import { Response } from 'express';
-
-import { ApiRestResponse } from 'src/engine/api/rest/types/api-rest-response.type';
-
 // https://gist.github.com/ManUtopiK/469aec75b655d6a4d912aeb3b75af3c9
-export const cleanGraphQLResponse = (input) => {
+export const cleanGraphQLResponse = (input: any) => {
   if (!input) return null;
   const output = {};
-  const isObject = (obj) => {
+  const isObject = (obj: any) => {
     return obj !== null && typeof obj === 'object' && !Array.isArray(obj);
   };
 
@@ -24,13 +20,3 @@ export const cleanGraphQLResponse = (input) => {
 
   return output;
 };
-
-export const handleResult = (res: Response, result: ApiRestResponse) => {
-  if (result.data.error) {
-    res
-      .status(result.data.status || 400)
-      .send({ error: `${result.data.error}` });
-  } else {
-    res.send(cleanGraphQLResponse(result.data));
-  }
-};
diff --git a/packages/twenty-server/src/engine/api/rest/api-rest.service.ts b/packages/twenty-server/src/engine/api/rest/api-rest.service.ts
@@ -1,87 +1,72 @@
-import { Injectable } from '@nestjs/common';
+import { BadRequestException, Injectable } from '@nestjs/common';
 import { HttpService } from '@nestjs/axios';
 
 import { Request } from 'express';
+import { AxiosResponse } from 'axios';
 
 import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
 import { ApiRestQueryBuilderFactory } from 'src/engine/api/rest/api-rest-query-builder/api-rest-query-builder.factory';
-import { TokenService } from 'src/engine/core-modules/auth/services/token.service';
-import { ApiRestResponse } from 'src/engine/api/rest/types/api-rest-response.type';
 import { ApiRestQuery } from 'src/engine/api/rest/types/api-rest-query.type';
 import { getServerUrl } from 'src/utils/get-server-url';
 
 @Injectable()
 export class ApiRestService {
   constructor(
-    private readonly tokenService: TokenService,
     private readonly environmentService: EnvironmentService,
     private readonly apiRestQueryBuilderFactory: ApiRestQueryBuilderFactory,
     private readonly httpService: HttpService,
   ) {}
 
-  async callGraphql(
-    request: Request,
-    data: ApiRestQuery,
-  ): Promise<ApiRestResponse> {
+  async callGraphql(request: Request, data: ApiRestQuery) {
     const baseUrl = getServerUrl(
       request,
       this.environmentService.get('SERVER_URL'),
     );
+    let response: AxiosResponse;
 
     try {
-      return await this.httpService.axiosRef.post(`${baseUrl}/graphql`, data, {
-        headers: {
-          'Content-Type': 'application/json',
-          Authorization: request.headers.authorization,
+      response = await this.httpService.axiosRef.post(
+        `${baseUrl}/graphql`,
+        data,
+        {
+          headers: {
+            'Content-Type': 'application/json',
+            Authorization: request.headers.authorization,
+          },
         },
-      });
+      );
     } catch (err) {
-      return {
-        data: {
-          error: `${err}. Please check your query.`,
-          status: err.response.status,
-        },
-      };
+      throw new BadRequestException(err.response.data);
+    }
+
+    if (response.data.errors?.length) {
+      throw new BadRequestException(response.data);
     }
+
+    return response;
   }
 
-  async get(request: Request): Promise<ApiRestResponse> {
-    try {
-      const data = await this.apiRestQueryBuilderFactory.get(request);
+  async get(request: Request) {
+    const data = await this.apiRestQueryBuilderFactory.get(request);
 
-      return await this.callGraphql(request, data);
-    } catch (err) {
-      return { data: { error: err, status: err.status } };
-    }
+    return await this.callGraphql(request, data);
   }
 
-  async delete(request: Request): Promise<ApiRestResponse> {
-    try {
-      const data = await this.apiRestQueryBuilderFactory.delete(request);
+  async delete(request: Request) {
+    const data = await this.apiRestQueryBuilderFactory.delete(request);
 
-      return await this.callGraphql(request, data);
-    } catch (err) {
-      return { data: { error: err, status: err.status } };
-    }
+    return await this.callGraphql(request, data);
   }
 
-  async create(request: Request): Promise<ApiRestResponse> {
-    try {
-      const data = await this.apiRestQueryBuilderFactory.create(request);
+  async create(request: Request) {
+    const data = await this.apiRestQueryBuilderFactory.create(request);
 
-      return await this.callGraphql(request, data);
-    } catch (err) {
-      return { data: { error: err, status: err.status } };
-    }
+    return await this.callGraphql(request, data);
   }
 
-  async update(request: Request): Promise<ApiRestResponse> {
-    try {
-      const data = await this.apiRestQueryBuilderFactory.update(request);
+  async update(request: Request) {
+    const data = await this.apiRestQueryBuilderFactory.update(request);
 
-      return await this.callGraphql(request, data);
-    } catch (err) {
-      return { data: { error: err, status: err.status } };
-    }
+    return await this.callGraphql(request, data);
   }
 }
diff --git a/packages/twenty-server/src/engine/api/rest/metadata-rest.controller.ts b/packages/twenty-server/src/engine/api/rest/metadata-rest.controller.ts
@@ -2,30 +2,38 @@ import { Controller, Get, Delete, Post, Put, Req, Res } from '@nestjs/common';
 
 import { Request, Response } from 'express';
 
-import { handleResult } from 'src/engine/api/rest/api-rest.controller.utils';
 import { ApiRestMetadataService } from 'src/engine/api/rest/metadata-rest.service';
+import { cleanGraphQLResponse } from 'src/engine/api/rest/api-rest.controller.utils';
 
 @Controller('rest/metadata/*')
 export class ApiRestMetadataController {
   constructor(private readonly apiRestService: ApiRestMetadataService) {}
 
   @Get()
   async handleApiGet(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.get(request));
+    const result = await this.apiRestService.get(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 
   @Delete()
   async handleApiDelete(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.delete(request));
+    const result = await this.apiRestService.delete(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 
   @Post()
   async handleApiPost(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.create(request));
+    const result = await this.apiRestService.create(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 
   @Put()
   async handleApiPut(@Req() request: Request, @Res() res: Response) {
-    handleResult(res, await this.apiRestService.update(request));
+    const result = await this.apiRestService.update(request);
+
+    res.send(cleanGraphQLResponse(result.data));
   }
 }
diff --git a/packages/twenty-server/src/engine/api/rest/types/api-rest-response.type.ts b/packages/twenty-server/src/engine/api/rest/types/api-rest-response.type.ts