Merge pull request #599 from looker-open-source/jonting/driveError

Add error info to google drive and fix dependencies
looker-open-source · Jun 6, 2024 · 1adbb8a · 1adbb8a
2 parents b69748c + dcb467a
commit 1adbb8a
Show file tree

Hide file tree

Showing 10 changed files with 163 additions and 10 deletions.
diff --git a/lib/actions/google/drive/google_drive.js b/lib/actions/google/drive/google_drive.js
@@ -13,7 +13,9 @@ exports.GoogleDriveAction = void 0;
 const https = require("request-promise-native");
 const googleapis_1 = require("googleapis");
 const winston = require("winston");
+const http_errors_1 = require("../../../error_types/http_errors");
 const Hub = require("../../../hub");
+const action_response_1 = require("../../../hub/action_response");
 class GoogleDriveAction extends Hub.OAuthAction {
     constructor() {
         super(...arguments);
@@ -42,19 +44,34 @@ class GoogleDriveAction extends Hub.OAuthAction {
                 const drive = yield this.driveClientFromRequest(stateJson.redirect, stateJson.tokens);
                 const filename = request.formParams.filename || request.suggestedFilename();
                 if (!filename) {
+                    const error = {
+                        http_code: http_errors_1.HTTP_ERROR.bad_request.code,
+                        status_code: http_errors_1.HTTP_ERROR.bad_request.status,
+                        message: `${http_errors_1.HTTP_ERROR.bad_request.description} Error creating filename from request`,
+                        location: "ActionContainer",
+                        documentation_url: "TODO",
+                    };
                     resp.success = false;
-                    resp.message = "Error creating filename";
-                    winston.error("Error creating filename");
+                    resp.error = error;
+                    resp.message = error.message;
+                    resp.webhookId = request.webhookId;
+                    winston.error(`${error.message}`, { error, webhookId: request.webhookId });
                     return resp;
                 }
                 try {
                     yield this.sendData(filename, request, drive);
                     resp.success = true;
                 }
                 catch (e) {
+                    let error = (0, action_response_1.errorWith)(http_errors_1.HTTP_ERROR.internal, "Error while sending data " + e.message);
+                    if (e.code && e.errors && e.errors[0] && e.errors[0].message) {
+                        error = Object.assign(Object.assign({}, error), { http_code: e.code, message: `${http_errors_1.HTTP_ERROR.internal.description} ${e.errors[0].message}` });
+                    }
                     resp.success = false;
                     resp.message = e.message;
-                    winston.error("Error while sending data " + e.message);
+                    resp.webhookId = request.webhookId;
+                    resp.error = error;
+                    winston.error(`${error.message}`, { error, webhookId: request.webhookId });
                 }
             }
             else {

diff --git a/lib/error_types/http_errors.d.ts b/lib/error_types/http_errors.d.ts
@@ -70,3 +70,8 @@ export declare const HTTP_ERROR: {
         description: string;
     };
 };
+export interface HttpErrorInfo {
+    status: string;
+    code: number;
+    description: string;
+}
diff --git a/lib/hub/action_response.d.ts b/lib/hub/action_response.d.ts
@@ -1,3 +1,4 @@
+import { HttpErrorInfo } from "../error_types/http_errors";
 import { ActionState } from "./action_state";
 export interface ValidationError {
     field: string;
@@ -10,6 +11,7 @@ export interface Error {
     location: string;
     documentation_url: string;
 }
+export declare function errorWith(errorInfo: HttpErrorInfo, message: string): Error;
 export declare class ActionResponse {
     message?: string;
     refreshQuery: boolean;

diff --git a/lib/hub/action_response.js b/lib/hub/action_response.js
@@ -1,6 +1,17 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
-exports.ActionResponse = void 0;
+exports.ActionResponse = exports.errorWith = void 0;
+function errorWith(errorInfo, message) {
+    const error = {
+        http_code: errorInfo.code,
+        status_code: errorInfo.status,
+        message: `${errorInfo.description} ${message}`,
+        location: "ActionContainer",
+        documentation_url: "TODO",
+    };
+    return error;
+}
+exports.errorWith = errorWith;
 class ActionResponse {
     constructor(fields) {
         this.refreshQuery = false;

diff --git a/package.json b/package.json
@@ -118,7 +118,7 @@
     "**/follow-redirects": "^1.14.8",
     "**/ssh2": "^1.4.0",
     "**/axios": "^1.2.1",
-    "**/google-auth-library": "^7.14.0",
+    "**/google-auth-library": "7.14.1",
     "**/pac-resolver": "^5.0.0",
     "**/mem": "^4.0.0",
     "**/formidable": "^3.2.4"

diff --git a/src/actions/google/drive/google_drive.ts b/src/actions/google/drive/google_drive.ts
@@ -5,7 +5,9 @@ import {Credentials, OAuth2Client} from "google-auth-library"
 import { drive_v3, google } from "googleapis"
 
 import * as winston from "winston"
+import { HTTP_ERROR } from "../../../error_types/http_errors"
 import * as Hub from "../../../hub"
+import { Error, errorWith } from "../../../hub/action_response"
 import Drive = drive_v3.Drive
 
 export class GoogleDriveAction extends Hub.OAuthAction {
@@ -36,18 +38,38 @@ export class GoogleDriveAction extends Hub.OAuthAction {
 
       const filename = request.formParams.filename || request.suggestedFilename()
       if (!filename) {
+        const error: Error = {
+          http_code: HTTP_ERROR.bad_request.code,
+          status_code: HTTP_ERROR.bad_request.status,
+          message: `${HTTP_ERROR.bad_request.description} Error creating filename from request`,
+          location: "ActionContainer",
+          documentation_url: "TODO",
+        }
         resp.success = false
-        resp.message = "Error creating filename"
-        winston.error("Error creating filename")
+        resp.error = error
+        resp.message = error.message
+        resp.webhookId = request.webhookId
+        winston.error(`${error.message}`, {error, webhookId: request.webhookId})
         return resp
       }
       try {
         await this.sendData(filename, request, drive)
         resp.success = true
       } catch (e: any) {
+        let error: Error = errorWith(
+            HTTP_ERROR.internal,
+            "Error while sending data " + e.message,
+        )
+
+        if (e.code && e.errors && e.errors[0] && e.errors[0].message) {
+          error = {...error, http_code: e.code, message: `${HTTP_ERROR.internal.description} ${e.errors[0].message}`}
+        }
+
         resp.success = false
         resp.message = e.message
-        winston.error("Error while sending data " + e.message)
+        resp.webhookId = request.webhookId
+        resp.error = error
+        winston.error(`${error.message}`, {error, webhookId: request.webhookId})
       }
     } else {
       resp.success = false

diff --git a/src/actions/google/drive/test_google_drive.ts b/src/actions/google/drive/test_google_drive.ts
@@ -88,6 +88,7 @@ describe(`${action.constructor.name} unit tests`, () => {
         state_url: "https://looker.state.url.com/action_hub_state/asdfasdfasdfasdf",
         state_json: JSON.stringify({tokens: "code", redirect: "url"}),
       }
+      request.webhookId = "webhookId"
       const stubClient = sinon.stub(action as any, "driveClientFromRequest")
         .resolves({
           files: {
@@ -98,11 +99,79 @@ describe(`${action.constructor.name} unit tests`, () => {
       chai.expect(resp).to.eventually.deep.equal({
         success: false,
         message: undefined,
-        // state: {data: "reset"},
         refreshQuery: false,
         validationErrors: [],
+        error: {
+          documentation_url: "TODO",
+          http_code: 500,
+          location: "ActionContainer",
+          message: "Internal server error. Error while sending data undefined",
+          status_code: "INTERNAL",
+        },
+        webhookId: "webhookId",
+      }).and.notify(stubClient.restore).and.notify(done)
+    })
+
+    it("sets state to reset if error in create contains code and reason", (done) => {
+      const request = new Hub.ActionRequest()
+      const dataBuffer = Buffer.from("Hello")
+      request.type = Hub.ActionType.Query
+      request.attachment = {dataBuffer, fileExtension: "csv"}
+      request.formParams = {filename: stubFileName, folder: stubFolder}
+      request.params = {
+        state_url: "https://looker.state.url.com/action_hub_state/asdfasdfasdfasdf",
+        state_json: JSON.stringify({tokens: "code", redirect: "url"}),
+      }
+      request.webhookId = "webhookId"
+      const stubClient = sinon.stub(action as any, "driveClientFromRequest")
+          .resolves({
+            files: {
+              create: async () => Promise.reject({
+                code: 1234,
+                errors: [
+                  {
+                    message: "testReason",
+                  },
+                ],
+              }),
+            },
+          })
+      const resp = action.validateAndExecute(request)
+      chai.expect(resp).to.eventually.deep.equal({
+        success: false,
+        message: undefined,
+        refreshQuery: false,
+        validationErrors: [],
+        error: {
+          documentation_url: "TODO",
+          http_code: 1234,
+          location: "ActionContainer",
+          message: "Internal server error. testReason",
+          status_code: "INTERNAL",
+        },
+        webhookId: "webhookId",
       }).and.notify(stubClient.restore).and.notify(done)
     })
+
+    it("filename missing in request", () => {
+      const request = new TestActionRequest()
+      request.webhookId = "webhookId"
+      const resp = action.validateAndExecute(request)
+      chai.expect(resp).to.eventually
+          .deep.equal({
+        message: "Server cannot process request due to client request error. Error creating filename from request",
+        refreshQuery: false,
+        success: false,
+        error: {
+          http_code: 400,
+          status_code: "BAD_REQUEST",
+          message: "Server cannot process request due to client request error. Error creating filename from request",
+          location: "ActionContainer",
+          documentation_url: "TODO",
+        },
+        webhookId: "webhookId",
+      })
+    })
   })
 
   describe("form", () => {
@@ -371,3 +440,9 @@ describe(`${action.constructor.name} unit tests`, () => {
     })
   })
 })
+
+class TestActionRequest extends Hub.ActionRequest {
+  suggestedFileName() {
+    return null
+  }
+}
diff --git a/src/error_types/http_errors.ts b/src/error_types/http_errors.ts
@@ -71,3 +71,12 @@ export const HTTP_ERROR = {
     description: "Server is acting as a gateway and cannot get a response in time.",
   },
 }
+
+export interface HttpErrorInfo {
+  /* Status name */
+  status: string,
+  /* Http status code to be returned */
+  code: number,
+  /* Generic description of the issue */
+  description: string,
+}
diff --git a/src/hub/action_response.ts b/src/hub/action_response.ts
@@ -1,3 +1,4 @@
+import {HttpErrorInfo} from "../error_types/http_errors"
 import {ActionState} from "./action_state"
 
 export interface ValidationError {
@@ -18,6 +19,17 @@ export interface Error {
   documentation_url: string
 }
 
+export function errorWith(errorInfo: HttpErrorInfo, message: string) {
+  const error: Error = {
+    http_code: errorInfo.code,
+    status_code: errorInfo.status,
+    message: `${errorInfo.description} ${message}`,
+    location: "ActionContainer",
+    documentation_url: "TODO",
+  }
+  return error
+}
+
 export class ActionResponse {
   message?: string
   refreshQuery = false

diff --git a/yarn.lock b/yarn.lock
@@ -3096,7 +3096,7 @@ glogg@^1.0.0:
   dependencies:
     sparkles "^1.0.0"
 
-google-auth-library@^1.3.1, google-auth-library@^6.0.0, google-auth-library@^7.0.0, google-auth-library@^7.14.0:
+google-auth-library@7.14.1, google-auth-library@^1.3.1, google-auth-library@^6.0.0, google-auth-library@^7.0.0, google-auth-library@^7.14.0:
   version "7.14.1"
   resolved "https://registry.yarnpkg.com/google-auth-library/-/google-auth-library-7.14.1.tgz#e3483034162f24cc71b95c8a55a210008826213c"
   integrity sha512-5Rk7iLNDFhFeBYc3s8l1CqzbEBcdhwR193RlD4vSNFajIcINKI8W8P0JLmBpwymHqqWbX34pJDQu39cSy/6RsA==