From 21e1f2a9fadd12d5b6c34ba8aec92be6d0d1e9d3 Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Tue, 8 Oct 2024 11:49:53 +0530
Subject: [PATCH] refactor: GUI error handling (#9528)

* refactor: error reporting

Signed-off-by: Pranav C <pranavxc@gmail.com>

* refactor: handle ee part

Signed-off-by: Pranav C <pranavxc@gmail.com>

* refactor: coderabbit review comments

Signed-off-by: Pranav C <pranavxc@gmail.com>

* refactor: linting

Signed-off-by: Pranav C <pranavxc@gmail.com>

* refactor: remove duplicate error log

Signed-off-by: Pranav C <pranavxc@gmail.com>

* refactor: request type correction

Signed-off-by: Pranav C <pranavxc@gmail.com>

---------

Signed-off-by: Pranav C <pranavxc@gmail.com>
---
 .../nc-gui/components/nc/ErrorBoundary.vue    |  3 +-
 .../nc-gui/composables/useGlobal/types.ts     |  1 +
 packages/nc-gui/nuxt-shim.d.ts                |  2 +
 packages/nc-gui/plugins/error-reporting.ts    | 92 +++++++++++++++++++
 packages/nc-gui/plugins/sentry.ts             | 15 +--
 packages/nocodb-sdk/src/lib/Api.ts            | 25 +++++
 .../src/controllers/utils.controller.ts       | 20 +++-
 packages/nocodb/src/schema/swagger.json       | 42 +++++++++
 packages/nocodb/src/services/utils.service.ts | 20 +++-
 9 files changed, 210 insertions(+), 10 deletions(-)
 create mode 100644 packages/nc-gui/plugins/error-reporting.ts

diff --git a/packages/nc-gui/components/nc/ErrorBoundary.vue b/packages/nc-gui/components/nc/ErrorBoundary.vue
index a80aec396c..c36321dc64 100644
--- a/packages/nc-gui/components/nc/ErrorBoundary.vue
+++ b/packages/nc-gui/components/nc/ErrorBoundary.vue
@@ -1,7 +1,6 @@
 <script lang="ts">
 // modified version of default NuxtErrorBoundary component - https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/app/components/nuxt-error-boundary.ts
 import { message } from 'ant-design-vue'
-import * as Sentry from '@sentry/vue'
 
 const MESSAGE_KEY = 'ErrorMessageKey'
 
@@ -54,7 +53,7 @@ export default {
         }, 30000)
 
         try {
-          Sentry.captureException(err)
+          nuxtApp.$report(err)
         } catch {
           // ignore
         }
diff --git a/packages/nc-gui/composables/useGlobal/types.ts b/packages/nc-gui/composables/useGlobal/types.ts
index 3d34b48bb1..9946ea88cb 100644
--- a/packages/nc-gui/composables/useGlobal/types.ts
+++ b/packages/nc-gui/composables/useGlobal/types.ts
@@ -40,6 +40,7 @@ export interface AppInfo {
   samlProviderName: string | null
   giftUrl: string
   feedEnabled: boolean
+  sentryDSN: string
 }
 
 export interface StoredState {
diff --git a/packages/nc-gui/nuxt-shim.d.ts b/packages/nc-gui/nuxt-shim.d.ts
index d16d5ad759..4262fab679 100644
--- a/packages/nc-gui/nuxt-shim.d.ts
+++ b/packages/nc-gui/nuxt-shim.d.ts
@@ -12,6 +12,8 @@ declare module '#app' {
     }
     /** {@link import('./plugins/tele') Telemetry} Emit telemetry event */
     $e: (event: string, data?: any) => void
+    /** {@link import('./plugins/report') Error reporting} Error reporting */
+    $report: (event: Error) => void
     $state: UseGlobalReturn
     $poller: {
       subscribe(
diff --git a/packages/nc-gui/plugins/error-reporting.ts b/packages/nc-gui/plugins/error-reporting.ts
new file mode 100644
index 0000000000..8dac9a0571
--- /dev/null
+++ b/packages/nc-gui/plugins/error-reporting.ts
@@ -0,0 +1,92 @@
+import * as Sentry from '@sentry/vue'
+import type { Api } from 'nocodb-sdk'
+
+class ErrorReporting {
+  errors: Error[] = []
+
+  // debounce error reporting to avoid sending multiple reports for the same error
+  private report = useDebounceFn(
+    () => {
+      try {
+        const errors = this.errors
+          // filter out duplicate errors and only include 2 lines of stack trace
+          .filter((error, index, self) => index === self.findIndex((t) => t.message === error.message))
+          .map((error) => ({
+            message: error.message,
+            stack: error.stack?.split('\n').slice(0, 2).join('\n'),
+          }))
+        this.errors = []
+        this.$api.utils.errorReport({ errors, extra: {} })
+      } catch {
+        // ignore
+      }
+    },
+    3000,
+    {
+      maxWait: 10000,
+    },
+  )
+
+  constructor(private $api: Api<unknown>) {}
+
+  // collect error to report later
+  collect(error: Error) {
+    this.errors.push(error)
+    // report errors after 3 seconds
+    this.report()
+  }
+}
+
+export default defineNuxtPlugin((nuxtApp) => {
+  if (isEeUI) {
+    nuxtApp.provide('report', function (error: Error) {
+      try {
+        Sentry.captureException(error)
+      } catch {
+        // ignore
+      }
+    })
+    return
+  }
+
+  const env = process.env.NODE_ENV === 'production' ? 'production' : 'development'
+  let isSentryConfigured = false
+  let isErrorReportingEnabled = false
+  let errorReporting: ErrorReporting | null = null
+
+  // load error reporting only if enabled and sentryDSN is not provided
+  watch(
+    [
+      () => (nuxtApp.$state as ReturnType<typeof useGlobal>).appInfo?.value?.errorReportingEnabled,
+      () => (nuxtApp.$state as ReturnType<typeof useGlobal>).appInfo?.value?.sentryDSN,
+    ],
+    ([enabled, sentryDSN]) => {
+      isSentryConfigured = enabled && !!sentryDSN
+      isErrorReportingEnabled = enabled
+      if (enabled && !sentryDSN) {
+        errorReporting = new ErrorReporting(nuxtApp.$api as Api<unknown>)
+      } else {
+        errorReporting = null
+      }
+    },
+    { immediate: true },
+  )
+
+  function report(error: Error) {
+    if (process.env.CI || process.env.PLAYWRIGHT) {
+      return
+    }
+
+    if (env !== 'production' && !process.env.NC_ENABLE_DEV_SENTRY) {
+      return
+    }
+
+    if (isSentryConfigured) {
+      Sentry.captureException(error)
+    } else if (isErrorReportingEnabled) {
+      errorReporting?.collect(error)
+    }
+  }
+
+  nuxtApp.provide('report', report)
+})
diff --git a/packages/nc-gui/plugins/sentry.ts b/packages/nc-gui/plugins/sentry.ts
index 939c0f1d46..871aba4cbb 100644
--- a/packages/nc-gui/plugins/sentry.ts
+++ b/packages/nc-gui/plugins/sentry.ts
@@ -20,14 +20,14 @@ export default defineNuxtPlugin((nuxtApp) => {
 
   let initialized = false
 
-  const init = () => {
+  const init = (dsn: string) => {
     // prevent multiple init
     if (initialized) return
     initialized = true
 
     Sentry.init({
       app: [vueApp],
-      dsn: 'https://64cb4904bcbec03a1b9a0be02a2d10a9@o4505953073889280.ingest.us.sentry.io/4507725383663616',
+      dsn,
       environment: env,
       integrations: [
         new Sentry.BrowserTracing({
@@ -56,11 +56,14 @@ export default defineNuxtPlugin((nuxtApp) => {
 
   // load sentry only if enabled
   watch(
-    () => (nuxtApp.$state as ReturnType<typeof useGlobal>).appInfo?.value?.errorReportingEnabled,
-    (enabled) => {
+    [
+      () => (nuxtApp.$state as ReturnType<typeof useGlobal>).appInfo?.value?.errorReportingEnabled,
+      () => (nuxtApp.$state as ReturnType<typeof useGlobal>).appInfo?.value?.sentryDSN,
+    ],
+    ([enabled, sentryDSN]) => {
       try {
-        if (enabled) init()
-      } catch (e) {
+        if (enabled && sentryDSN) init(sentryDSN)
+      } catch {
         // ignore
       }
     },
diff --git a/packages/nocodb-sdk/src/lib/Api.ts b/packages/nocodb-sdk/src/lib/Api.ts
index 18a7abf1da..f98a214ae4 100644
--- a/packages/nocodb-sdk/src/lib/Api.ts
+++ b/packages/nocodb-sdk/src/lib/Api.ts
@@ -3100,6 +3100,14 @@ export interface CalendarColumnReqType {
   order?: number;
 }
 
+export interface ErrorReportReqType {
+  errors?: {
+    message?: string;
+    stack?: string;
+  }[];
+  extra?: object;
+}
+
 /**
  * Model for Comment
  */
@@ -10735,6 +10743,23 @@ export class Api<
       }),
 
     /**
+     * @description Error Reporting
+     *
+     * @tags Utils, Internal
+     * @name ErrorReport
+     * @summary Error Reporting
+     * @request POST:/api/v1/error-reporting
+     */
+    errorReport: (data: any, params: RequestParams = {}) =>
+      this.request<any, any>({
+        path: `/api/v1/error-reporting`,
+        method: 'POST',
+        body: data,
+        type: ContentType.Json,
+        ...params,
+      }),
+
+    /**
  * @description Generic Axios Call
  * 
  * @tags Utils
diff --git a/packages/nocodb/src/controllers/utils.controller.ts b/packages/nocodb/src/controllers/utils.controller.ts
index c68b00f3f7..682693c8e6 100644
--- a/packages/nocodb/src/controllers/utils.controller.ts
+++ b/packages/nocodb/src/controllers/utils.controller.ts
@@ -12,6 +12,7 @@ import {
 } from '@nestjs/common';
 import { ProjectRoles, validateAndExtractSSLProp } from 'nocodb-sdk';
 import {
+  ErrorReportReqType,
   getTestDatabaseName,
   IntegrationsType,
   OrgUserRoles,
@@ -26,7 +27,7 @@ import { NcRequest } from '~/interface/config';
 import { Integration } from '~/models';
 import { MetaTable, RootScopes } from '~/utils/globals';
 import { NcError } from '~/helpers/catchError';
-import { deepMerge } from '~/utils';
+import { deepMerge, isEE } from '~/utils';
 import Noco from '~/Noco';
 
 @Controller()
@@ -173,4 +174,21 @@ export class UtilsController {
   async feed(@Request() req: NcRequest) {
     return await this.utilsService.feed(req);
   }
+
+  @UseGuards(PublicApiLimiterGuard)
+  @Post('/api/v1/error-reporting')
+  async reportErrors(@Req() req: NcRequest, @Body() body: ErrorReportReqType) {
+    if (
+      `${process.env.NC_DISABLE_ERR_REPORTS}` === 'true' ||
+      isEE ||
+      process.env.NC_SENTRY_DSN
+    ) {
+      return {};
+    }
+
+    return (await this.utilsService.reportErrors({
+      req,
+      body,
+    })) as any;
+  }
 }
diff --git a/packages/nocodb/src/schema/swagger.json b/packages/nocodb/src/schema/swagger.json
index 5d236a25af..9d4c20e376 100644
--- a/packages/nocodb/src/schema/swagger.json
+++ b/packages/nocodb/src/schema/swagger.json
@@ -15843,6 +15843,26 @@
         ]
       }
     },
+    "/api/v1/error-reporting": {
+      "post": {
+        "summary": "Error Reporting",
+        "operationId": "utils-error-report",
+        "responses": {
+        },
+        "description": "Error Reporting",
+        "tags": [
+          "Utils",
+          "Internal"
+        ],
+        "requestBody": {
+          "content": {
+            "application/json": {
+              "$ref": "#/components/schemas/ErrorReportReq"
+            }
+          }
+        }
+      }
+    },
     "/api/v1/db/meta/axiosRequestMake": {
       "parameters": [
         {
@@ -27014,6 +27034,28 @@
           "id": "psbv6c6y9qvbu"
         }
       },
+      "ErrorReportReq": {
+        "type": "object",
+        "properties": {
+          "errors": {
+            "type": "array",
+            "items": {
+              "type": "object",
+              "properties": {
+                "message": {
+                  "type": "string"
+                },
+                "stack": {
+                  "type": "string"
+                }
+              }
+            }
+          },
+          "extra": {
+            "type": "object"
+          }
+        }
+      },
       "Comment": {
         "description": "Model for Comment",
         "type": "object",
diff --git a/packages/nocodb/src/services/utils.service.ts b/packages/nocodb/src/services/utils.service.ts
index bb11381ec3..0079ccc83a 100644
--- a/packages/nocodb/src/services/utils.service.ts
+++ b/packages/nocodb/src/services/utils.service.ts
@@ -6,13 +6,14 @@ import { ViewTypes } from 'nocodb-sdk';
 import { ConfigService } from '@nestjs/config';
 import { useAgent } from 'request-filtering-agent';
 import dayjs from 'dayjs';
+import type { ErrorReportReqType } from 'nocodb-sdk';
 import type { AppConfig, NcRequest } from '~/interface/config';
-import { T } from '~/utils';
 import { NC_APP_SETTINGS, NC_ATTACHMENT_FIELD_SIZE } from '~/constants';
 import SqlMgrv2 from '~/db/sql-mgr/v2/SqlMgrv2';
 import { NcError } from '~/helpers/catchError';
 import { Base, Store, User } from '~/models';
 import Noco from '~/Noco';
+import { T } from '~/utils';
 import NcConnectionMgrv2 from '~/utils/common/NcConnectionMgrv2';
 import getInstance from '~/utils/getInstance';
 import { CacheScope, MetaTable, RootScopes } from '~/utils/globals';
@@ -456,6 +457,10 @@ export class UtilsService {
       ncMin: !!process.env.NC_MIN,
       teleEnabled: process.env.NC_DISABLE_TELE !== 'true',
       errorReportingEnabled: process.env.NC_DISABLE_ERR_REPORTS !== 'true',
+      sentryDSN:
+        process.env.NC_DISABLE_ERR_REPORTS !== 'true'
+          ? process.env.NC_SENTRY_DSN
+          : null,
       auditEnabled: process.env.NC_DISABLE_AUDIT !== 'true',
       ncSiteUrl: (param.req as any).ncSiteUrl,
       ee: Noco.isEE(),
@@ -480,6 +485,19 @@ export class UtilsService {
     return result;
   }
 
+  async reportErrors(param: { body: ErrorReportReqType; req: NcRequest }) {
+    for (const error of param.body?.errors ?? []) {
+      T.emit('evt', {
+        evt_type: 'gui:error',
+        properties: {
+          message: error.message,
+          stack: error.stack?.split('\n').slice(0, 2).join('\n'),
+          ...(param.body.extra || {}),
+        },
+      });
+    }
+  }
+
   async feed(req: NcRequest) {
     const {
       type = 'all',