From dcbaeffe7e6daca24c23865ecf5565fe2ad559fb Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Mon, 4 Mar 2024 13:27:33 +0000
Subject: [PATCH 1/2] fix: handle group by load method exceptions

---
 packages/nc-gui/composables/useViewGroupBy.ts | 250 +++++++++---------
 1 file changed, 130 insertions(+), 120 deletions(-)

diff --git a/packages/nc-gui/composables/useViewGroupBy.ts b/packages/nc-gui/composables/useViewGroupBy.ts
index 739b33fcc4..8f3480e92d 100644
--- a/packages/nc-gui/composables/useViewGroupBy.ts
+++ b/packages/nc-gui/composables/useViewGroupBy.ts
@@ -1,5 +1,7 @@
 import { type ColumnType, type SelectOptionsType, UITypes, type ViewType } from 'nocodb-sdk'
 import type { Ref } from 'vue'
+import { message } from 'ant-design-vue'
+import { extractSdkResponseErrorMsg } from '../utils'
 import { GROUP_BY_VARS, ref, storeToRefs, useApi, useBase, useViewColumnsOrThrow } from '#imports'
 import type { Group, GroupNestedIn, Row } from '#imports'
 
@@ -178,152 +180,160 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
   }
 
   async function loadGroups(params: any = {}, group?: Group) {
-    group = group || rootGroup.value
+    try {
+      group = group || rootGroup.value
 
-    if (!base?.value?.id || !view.value?.id || !view.value?.fk_model_id || !group) return
+      if (!base?.value?.id || !view.value?.id || !view.value?.fk_model_id || !group) return
 
-    if (groupBy.value.length === 0) {
-      group.children = []
-      return
-    }
+      if (groupBy.value.length === 0) {
+        group.children = []
+        return
+      }
 
-    if (group.nestedIn.length > groupBy.value.length) return
+      if (group.nestedIn.length > groupBy.value.length) return
 
-    if (group.nestedIn.length === 0) nextGroupColor.value = colors.value[0]
-    const groupby = groupBy.value[group.nestedIn.length]
+      if (group.nestedIn.length === 0) nextGroupColor.value = colors.value[0]
+      const groupby = groupBy.value[group.nestedIn.length]
 
-    const nestedWhere = calculateNestedWhere(group.nestedIn, where?.value)
-    if (!groupby || !groupby.column.title) return
+      const nestedWhere = calculateNestedWhere(group.nestedIn, where?.value)
+      if (!groupby || !groupby.column.title) return
 
-    if (isPublic.value && !sharedView.value?.uuid) {
-      return
-    }
+      if (isPublic.value && !sharedView.value?.uuid) {
+        return
+      }
 
-    const response = !isPublic.value
-      ? await api.dbViewRow.groupBy('noco', base.value.id, view.value.fk_model_id, view.value.id, {
-          offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByGroupLimit.value),
-          limit: group.paginationData.pageSize ?? groupByGroupLimit.value,
-          ...params,
-          ...(isUIAllowed('sortSync') ? {} : { sortArrJson: JSON.stringify(sorts.value) }),
-          ...(isUIAllowed('filterSync') ? {} : { filterArrJson: JSON.stringify(nestedFilters.value) }),
-          where: `${nestedWhere}`,
-          sort: `${groupby.sort === 'desc' ? '-' : ''}${groupby.column.title}`,
-          column_name: groupby.column.title,
-        } as any)
-      : await api.public.dataGroupBy(sharedView.value!.uuid!, {
-          offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByGroupLimit.value),
-          limit: group.paginationData.pageSize ?? groupByGroupLimit.value,
-          ...params,
-          where: nestedWhere,
-          sort: `${groupby.sort === 'desc' ? '-' : ''}${groupby.column.title}`,
-          column_name: groupby.column.title,
-          sortsArr: sorts.value,
-          filtersArr: nestedFilters.value,
-        })
+      const response = !isPublic.value
+        ? await api.dbViewRow.groupBy('noco', base.value.id, view.value.fk_model_id, view.value.id, {
+            offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByGroupLimit.value),
+            limit: group.paginationData.pageSize ?? groupByGroupLimit.value,
+            ...params,
+            ...(isUIAllowed('sortSync') ? {} : { sortArrJson: JSON.stringify(sorts.value) }),
+            ...(isUIAllowed('filterSync') ? {} : { filterArrJson: JSON.stringify(nestedFilters.value) }),
+            where: `${nestedWhere}`,
+            sort: `${groupby.sort === 'desc' ? '-' : ''}${groupby.column.title}`,
+            column_name: groupby.column.title,
+          } as any)
+        : await api.public.dataGroupBy(sharedView.value!.uuid!, {
+            offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByGroupLimit.value),
+            limit: group.paginationData.pageSize ?? groupByGroupLimit.value,
+            ...params,
+            where: nestedWhere,
+            sort: `${groupby.sort === 'desc' ? '-' : ''}${groupby.column.title}`,
+            column_name: groupby.column.title,
+            sortsArr: sorts.value,
+            filtersArr: nestedFilters.value,
+          })
 
-    const tempList: Group[] = response.list.reduce((acc: Group[], curr: Record<string, any>) => {
-      const keyExists = acc.find(
-        (a) => a.key === valueToTitle(curr[groupby.column.column_name!] ?? curr[groupby.column.title!], groupby.column),
-      )
-      if (keyExists) {
-        keyExists.count += +curr.count
-        keyExists.paginationData = { page: 1, pageSize: groupByGroupLimit.value, totalRows: keyExists.count }
-        return acc
-      }
-      if (groupby.column.title && groupby.column.uidt) {
-        acc.push({
-          key: valueToTitle(curr[groupby.column.title!], groupby.column),
-          column: groupby.column,
-          count: +curr.count,
-          color: findKeyColor(curr[groupby.column.title!], groupby.column),
-          nestedIn: [
-            ...group!.nestedIn,
-            {
-              title: groupby.column.title,
-              column_name: groupby.column.title!,
-              key: valueToTitle(curr[groupby.column.title!], groupby.column),
-              column_uidt: groupby.column.uidt,
+      const tempList: Group[] = response.list.reduce((acc: Group[], curr: Record<string, any>) => {
+        const keyExists = acc.find(
+          (a) => a.key === valueToTitle(curr[groupby.column.column_name!] ?? curr[groupby.column.title!], groupby.column),
+        )
+        if (keyExists) {
+          keyExists.count += +curr.count
+          keyExists.paginationData = { page: 1, pageSize: groupByGroupLimit.value, totalRows: keyExists.count }
+          return acc
+        }
+        if (groupby.column.title && groupby.column.uidt) {
+          acc.push({
+            key: valueToTitle(curr[groupby.column.title!], groupby.column),
+            column: groupby.column,
+            count: +curr.count,
+            color: findKeyColor(curr[groupby.column.title!], groupby.column),
+            nestedIn: [
+              ...group!.nestedIn,
+              {
+                title: groupby.column.title,
+                column_name: groupby.column.title!,
+                key: valueToTitle(curr[groupby.column.title!], groupby.column),
+                column_uidt: groupby.column.uidt,
+              },
+            ],
+            paginationData: {
+              page: 1,
+              pageSize: group!.nestedIn.length < groupBy.value.length - 1 ? groupByGroupLimit.value : groupByRecordLimit.value,
+              totalRows: +curr.count,
             },
-          ],
-          paginationData: {
-            page: 1,
-            pageSize: group!.nestedIn.length < groupBy.value.length - 1 ? groupByGroupLimit.value : groupByRecordLimit.value,
-            totalRows: +curr.count,
-          },
-          nested: group!.nestedIn.length < groupBy.value.length - 1,
-        })
-      }
-      return acc
-    }, [])
+            nested: group!.nestedIn.length < groupBy.value.length - 1,
+          })
+        }
+        return acc
+      }, [])
 
-    if (!group.children) group.children = []
+      if (!group.children) group.children = []
 
-    for (const temp of tempList) {
-      const keyExists = group.children?.find((a) => a.key === temp.key)
-      if (keyExists) {
-        temp.paginationData = {
-          page: keyExists.paginationData.page || temp.paginationData.page,
-          pageSize: keyExists.paginationData.pageSize || temp.paginationData.pageSize,
-          totalRows: temp.count,
+      for (const temp of tempList) {
+        const keyExists = group.children?.find((a) => a.key === temp.key)
+        if (keyExists) {
+          temp.paginationData = {
+            page: keyExists.paginationData.page || temp.paginationData.page,
+            pageSize: keyExists.paginationData.pageSize || temp.paginationData.pageSize,
+            totalRows: temp.count,
+          }
+          temp.color = keyExists.color
+          // update group
+          Object.assign(keyExists, temp)
+          continue
         }
-        temp.color = keyExists.color
-        // update group
-        Object.assign(keyExists, temp)
-        continue
+        group.children.push(temp)
       }
-      group.children.push(temp)
-    }
 
-    // clear rest of the children
-    group.children = group.children.filter((c) => tempList.find((t) => t.key === c.key))
+      // clear rest of the children
+      group.children = group.children.filter((c) => tempList.find((t) => t.key === c.key))
 
-    if (group.count <= (group.paginationData.pageSize ?? groupByGroupLimit.value)) {
-      group.children.sort((a, b) => {
-        const orderA = tempList.findIndex((t) => t.key === a.key)
-        const orderB = tempList.findIndex((t) => t.key === b.key)
-        return orderA - orderB
-      })
-    }
+      if (group.count <= (group.paginationData.pageSize ?? groupByGroupLimit.value)) {
+        group.children.sort((a, b) => {
+          const orderA = tempList.findIndex((t) => t.key === a.key)
+          const orderB = tempList.findIndex((t) => t.key === b.key)
+          return orderA - orderB
+        })
+      }
 
-    group.paginationData = response.pageInfo
+      group.paginationData = response.pageInfo
 
-    // to cater the case like when querying with a non-zero offset
-    // the result page may point to the target page where the actual returned data don't display on
-    const expectedPage = Math.max(1, Math.ceil(group.paginationData.totalRows! / group.paginationData.pageSize!))
-    if (expectedPage < group.paginationData.page!) {
-      await groupWrapperChangePage(expectedPage, group)
+      // to cater the case like when querying with a non-zero offset
+      // the result page may point to the target page where the actual returned data don't display on
+      const expectedPage = Math.max(1, Math.ceil(group.paginationData.totalRows! / group.paginationData.pageSize!))
+      if (expectedPage < group.paginationData.page!) {
+        await groupWrapperChangePage(expectedPage, group)
+      }
+    } catch (e) {
+      message.error(await extractSdkResponseErrorMsg(e))
     }
   }
 
   async function loadGroupData(group: Group, force = false, params: any = {}) {
-    if (!base?.value?.id || !view.value?.id || !view.value?.fk_model_id) return
+    try {
+      if (!base?.value?.id || !view.value?.id || !view.value?.fk_model_id) return
 
-    if (group.children && !force) return
+      if (group.children && !force) return
 
-    if (!group.paginationData) {
-      group.paginationData = { page: 1, pageSize: groupByRecordLimit.value }
-    }
+      if (!group.paginationData) {
+        group.paginationData = { page: 1, pageSize: groupByRecordLimit.value }
+      }
 
-    const nestedWhere = calculateNestedWhere(group.nestedIn, where?.value)
+      const nestedWhere = calculateNestedWhere(group.nestedIn, where?.value)
 
-    const query = {
-      offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByRecordLimit.value),
-      limit: group.paginationData.pageSize ?? groupByRecordLimit.value,
-      where: `${nestedWhere}`,
-    }
+      const query = {
+        offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByRecordLimit.value),
+        limit: group.paginationData.pageSize ?? groupByRecordLimit.value,
+        where: `${nestedWhere}`,
+      }
 
-    const response = !isPublic.value
-      ? await api.dbViewRow.list('noco', base.value.id, view.value.fk_model_id, view.value.id, {
-          ...query,
-          ...params,
-          ...(isUIAllowed('sortSync') ? {} : { sortArrJson: JSON.stringify(sorts.value) }),
-          ...(isUIAllowed('filterSync') ? {} : { filterArrJson: JSON.stringify(nestedFilters.value) }),
-        } as any)
-      : await fetchSharedViewData({ sortsArr: sorts.value, filtersArr: nestedFilters.value, ...query })
-
-    group.count = response.pageInfo.totalRows ?? 0
-    group.rows = formatData(response.list)
-    group.paginationData = response.pageInfo
+      const response = !isPublic.value
+        ? await api.dbViewRow.list('noco', base.value.id, view.value.fk_model_id, view.value.id, {
+            ...query,
+            ...params,
+            ...(isUIAllowed('sortSync') ? {} : { sortArrJson: JSON.stringify(sorts.value) }),
+            ...(isUIAllowed('filterSync') ? {} : { filterArrJson: JSON.stringify(nestedFilters.value) }),
+          } as any)
+        : await fetchSharedViewData({ sortsArr: sorts.value, filtersArr: nestedFilters.value, ...query })
+
+      group.count = response.pageInfo.totalRows ?? 0
+      group.rows = formatData(response.list)
+      group.paginationData = response.pageInfo
+    } catch (e) {
+      message.error(await extractSdkResponseErrorMsg(e))
+    }
   }
 
   const loadGroupPage = async (group: Group, p: number) => {

From 05ce666df80eb49ccd19950283f776b5c0a08a0b Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Mon, 4 Mar 2024 13:27:34 +0000
Subject: [PATCH 2/2] fix: improved and custom version of nuxt errorboundary
 component

---
 packages/nc-gui/app.vue                       |  47 +-----
 .../nc-gui/components/general/FormBanner.vue  |   2 +-
 .../nc-gui/components/nc/ErrorBoundary.vue    | 146 ++++++++++++++++++
 .../nc-gui/components/smartsheet/Form.vue     |   4 +-
 4 files changed, 154 insertions(+), 45 deletions(-)
 create mode 100644 packages/nc-gui/components/nc/ErrorBoundary.vue

diff --git a/packages/nc-gui/app.vue b/packages/nc-gui/app.vue
index bf4941ab0c..13e8934fd4 100644
--- a/packages/nc-gui/app.vue
+++ b/packages/nc-gui/app.vue
@@ -1,6 +1,5 @@
 <script setup lang="ts">
-import { message } from 'ant-design-vue'
-import { extractSdkResponseErrorMsg } from './utils'
+import ErrorBoundary from './components/nc/ErrorBoundary.vue'
 import { applyNonSelectable, computed, isEeUI, isMac, useCommandPalette, useRouter, useTheme } from '#imports'
 import type { CommandPaletteType } from '~/lib'
 
@@ -98,49 +97,18 @@ onMounted(() => {
     refreshCommandPalette()
   })
 })
-
-let errorCount = 0
-
-const handleError = async (error, clearError) => {
-  console.error('UI ERROR', error.value)
-  // if error is api error, show toast message with error message
-  if (error.value?.response) {
-    message.warn(await extractSdkResponseErrorMsg(error.value))
-  } else {
-    // else show generic error message
-    message.warn('Something went wrong. Please reload the page if page is not functioning properly.')
-  }
-  clearError()
-
-  // if error count is more than 3 within 3 second, navigate to home
-  // since it's likely endless loop of errors due to some UI issue in certain page
-  errorCount++
-  if (errorCount > 3) {
-    router.push('/')
-  }
-
-  // reset error count after 1 second
-  setTimeout(() => {
-    errorCount = 0
-  }, 3000)
-}
 </script>
 
 <template>
   <a-config-provider>
     <NuxtLayout :name="disableBaseLayout ? false : 'base'">
-      <NuxtErrorBoundary>
+      <ErrorBoundary>
         <NuxtPage :key="key" :transition="false" />
-
-        <!-- on error, clear error and show toast message -->
-        <template #error="{ error, clearError }">
-          {{ handleError(error, clearError) }}
-        </template>
-      </NuxtErrorBoundary>
+      </ErrorBoundary>
     </NuxtLayout>
   </a-config-provider>
 
-  <NuxtErrorBoundary>
+  <ErrorBoundary>
     <div>
       <!-- Command Menu -->
       <CmdK
@@ -158,10 +126,5 @@ const handleError = async (error, clearError) => {
       <!-- Documentation. Integrated NocoDB Docs directly inside the Product -->
       <CmdJ />
     </div>
-
-    <!-- on error, clear error and show toast message -->
-    <template #error="{ error, clearError }">
-      {{ handleError(error, clearError) }}
-    </template>
-  </NuxtErrorBoundary>
+  </ErrorBoundary>
 </template>
diff --git a/packages/nc-gui/components/general/FormBanner.vue b/packages/nc-gui/components/general/FormBanner.vue
index 4904a80258..63bb6fe8c9 100644
--- a/packages/nc-gui/components/general/FormBanner.vue
+++ b/packages/nc-gui/components/general/FormBanner.vue
@@ -1,5 +1,5 @@
 <script lang="ts" setup>
-import type { AttachmentResType } from 'nocodb-sdk';
+import type { AttachmentResType } from 'nocodb-sdk'
 
 interface Props {
   bannerImageUrl?: AttachmentResType
diff --git a/packages/nc-gui/components/nc/ErrorBoundary.vue b/packages/nc-gui/components/nc/ErrorBoundary.vue
new file mode 100644
index 0000000000..25b7df4585
--- /dev/null
+++ b/packages/nc-gui/components/nc/ErrorBoundary.vue
@@ -0,0 +1,146 @@
+<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 { computed, onErrorCaptured, ref, useCopy, useNuxtApp } from '#imports'
+
+export default {
+  emits: {
+    error(_error: unknown) {
+      return true
+    },
+  },
+  setup(_props, { emit }) {
+    const nuxtApp = useNuxtApp()
+    const error = ref()
+    const prevError = ref()
+    const errModal = computed(() => !!error.value)
+    const key = ref(0)
+    const isErrorExpanded = ref(false)
+    const { copy } = useCopy()
+
+    onErrorCaptured((err) => {
+      if (import.meta.client && (!nuxtApp.isHydrating || !nuxtApp.payload.serverRendered)) {
+        console.log('UI Error :', err)
+        emit('error', err)
+        error.value = err
+        return false
+      }
+    })
+
+    const copyError = async () => {
+      try {
+        if (error.value) await copy(`message: ${error.value.message}\n\n${error.value.stack}`)
+        message.info('Error message copied to clipboard.')
+      } catch (e) {
+        message.error('Something went wrong while copying to clipboard, please copy from browser console.')
+      }
+    }
+
+    const reload = () => {
+      prevError.value = error.value
+      error.value = null
+      key.value++
+    }
+
+    const navigateToHome = () => {
+      prevError.value = error.value
+      error.value = null
+      location.hash = '/'
+      location.reload()
+    }
+
+    return {
+      errModal,
+      error,
+      key,
+      isErrorExpanded,
+      prevError,
+      copyError,
+      reload,
+      navigateToHome,
+    }
+  },
+}
+</script>
+
+<template>
+  <slot :key="key"></slot>
+  <slot name="error">
+    <NcModal v-model:visible="errModal" :class="{ active: errModal }" :centered="true" :closable="false" :footer="null">
+      <div class="w-full flex flex-col gap-1">
+        <h2 class="text-xl font-semibold">Oops! Something unexpected happened :/</h2>
+
+        <p class="mb-0">
+          <span
+            >Please report this error in our
+            <a href="https://discord.gg/8jX2GQn" target="_blank" rel="noopener noreferrer">Discord channel</a>. You can copy the
+            error message by clicking the "Copy" button below.</span
+          >
+        </p>
+
+        <span class="cursor-pointer" @click="isErrorExpanded = !isErrorExpanded"
+          >{{ isErrorExpanded ? 'Hide' : 'Show' }} details
+          <GeneralIcon
+            icon="arrowDown"
+            class="transition-transform transform duration-300"
+            :class="{
+              'rotate-180': isErrorExpanded,
+            }"
+        /></span>
+        <div
+          class="nc-error"
+          :class="{
+            active: isErrorExpanded,
+          }"
+        >
+          <div class="nc-left-vertical-bar"></div>
+          <div class="nc-error-content">
+            <span class="font-weight-bold">Message: {{ error.message }}</span>
+            <br />
+            <div class="text-gray-500 mt-2">{{ error.stack }}</div>
+          </div>
+        </div>
+        <div class="flex justify-end gap-2">
+          <NcButton size="small" type="secondary" @click="copyError">
+            <div class="flex items-center gap-1">
+              <GeneralIcon icon="copy" />
+              Copy Error
+            </div>
+          </NcButton>
+          <NcButton v-if="!prevError || error.message !== prevError.message" size="small" @click="reload">
+            <div class="flex items-center gap-1">
+              <GeneralIcon icon="reload" />
+              Reload
+            </div>
+          </NcButton>
+          <NcButton v-else size="small" @click="navigateToHome">
+            <div class="flex items-center gap-1">
+              <GeneralIcon icon="link" />
+              Home
+            </div>
+          </NcButton>
+        </div>
+      </div>
+    </NcModal>
+  </slot>
+</template>
+
+<style scoped lang="scss">
+.nc-error {
+  @apply flex gap-2 mb-2 max-h-0;
+  white-space: pre;
+  transition: max-height 300ms linear;
+
+  &.active {
+    max-height: 250px;
+  }
+
+  .nc-left-vertical-bar {
+    @apply w-6px min-w-6px rounded min-h-full bg-gray-300;
+  }
+
+  .nc-error-content {
+    @apply min-w-0 overflow-auto pl-2 flex-shrink;
+  }
+}
+</style>
diff --git a/packages/nc-gui/components/smartsheet/Form.vue b/packages/nc-gui/components/smartsheet/Form.vue
index af17811e86..9dbb379ce3 100644
--- a/packages/nc-gui/components/smartsheet/Form.vue
+++ b/packages/nc-gui/components/smartsheet/Form.vue
@@ -4,6 +4,7 @@ import tinycolor from 'tinycolor2'
 import { Pane, Splitpanes } from 'splitpanes'
 import 'splitpanes/dist/splitpanes.css'
 import {
+  type AttachmentResType,
   ProjectRoles,
   RelationTypes,
   UITypes,
@@ -12,7 +13,6 @@ import {
   isLinksOrLTAR,
   isSelectTypeCol,
   isVirtualCol,
-type AttachmentResType,
 } from 'nocodb-sdk'
 import type { Permission } from '#imports'
 import {
@@ -529,7 +529,7 @@ onChangeFile((files) => {
 
 const handleOnUploadImage = (data: AttachmentResType = null) => {
   if (imageCropperData.value.cropFor === 'banner') {
-    formViewData.value!.banner_image_url = data 
+    formViewData.value!.banner_image_url = data
   } else {
     formViewData.value!.logo_url = data
   }