Merge pull request #4828 from supabase/bug/studio-load-older-logs-empty-page

bugfix/studio/LogPage: fix for load older logs empty page bug.

studio/pages/project/[ref]/settings/logs/[type].tsx

@@ -3,7 +3,15 @@ import React, { useEffect, useRef, useState } from 'react'
 import { NextPage } from 'next'
 import { useRouter } from 'next/router'
 import { observer } from 'mobx-react-lite'
-import { Typography, IconLoader, IconAlertCircle, IconRewind, Button, IconInfo } from '@supabase/ui'
+import {
+  Typography,
+  IconLoader,
+  IconAlertCircle,
+  IconRewind,
+  Button,
+  IconInfo,
+  Card,
+} from '@supabase/ui'
 
 import { withAuth } from 'hooks'
 import { get } from 'lib/common/fetch'
@@ -72,6 +80,9 @@ export const LogPage: NextPage = () => {
     if (prevPageData === null) {
       // reduce interval window limit by using the timestamp of the last log
       queryParams = genQueryParams(params)
+    } else if (prevPageData.data.length === 0) {
+      // no rows returned, indicates that no more data to retrieve and append.
+      return null
     } else {
       const len = prevPageData.data.length
       const { timestamp: tsLimit }: LogData = prevPageData.data[len - 1]
@@ -84,18 +95,16 @@ export const LogPage: NextPage = () => {
   }
   const {
     data = [],
+    error: swrError,
     isValidating,
     mutate,
     size,
     setSize,
   } = useSWRInfinite<Logs>(getKeyLogs, get, { revalidateOnFocus: false })
-
-  // const { data, isValidating, mutate } = useSWR<Logs>(logUrl, get, { revalidateOnFocus: false })
   let logData: LogData[] = []
-  let error: null | string = null
-
+  let error: null | string = swrError ? swrError.message : null
   data.forEach((response: Logs) => {
-    if (response && response.data) {
+    if (!error && response && response.data) {
       logData = [...logData, ...response.data]
     }
     if (!error && response && response.error) {
@@ -220,9 +229,16 @@ export const LogPage: NextPage = () => {
             </div>
           )}
           {error && (
-            <div className="flex w-full h-full justify-center items-center space-x-2 mx-auto">
-              <IconAlertCircle size={16} />
-              <Typography.Text type="secondary">Sorry! Could not fetch data</Typography.Text>
+            <div className="flex w-full h-full justify-center items-center mx-auto">
+              <Card className="flex flex-col gap-y-2">
+                <div className="flex flex-row gap-x-2 py-2">
+                  <IconAlertCircle size={16} />
+                  <Typography.Text type="secondary">
+                    Sorry! An error occured when fetching data.
+                  </Typography.Text>
+                </div>
+                <Typography.Text type="warning">{error}</Typography.Text>
+              </Card>
             </div>
           )}
           <LogTable data={logData} isCustomQuery={mode === 'custom'} />

studio/tests/pages/projects/logs.test.js

@@ -42,15 +42,21 @@ LogPage.mockImplementation((props) => {
   const Page = jest.requireActual('pages/project/[ref]/settings/logs/[type]').LogPage
   // wrap with SWR to reset the cache each time
   return (
-    <SWRConfig value={{ provider: () => new Map() }}>
+    <SWRConfig
+      value={{
+        provider: () => new Map(),
+        shouldRetryOnError: false,
+      }}
+    >
       <Page {...props} />
     </SWRConfig>
   )
 })
 
-import { render, fireEvent, waitFor, screen } from '@testing-library/react'
+import { render, fireEvent, waitFor, screen, act } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { getToggleByText } from '../../helpers'
+import { wait } from '@testing-library/user-event/dist/utils'
 
 beforeEach(() => {
   // reset mocks between tests
@@ -298,3 +304,27 @@ test('load older btn will fetch older logs', async () => {
   })
   await waitFor(() => screen.getByText('second event'))
 })
+
+test('bug: load older btn does not error out when previous page is empty', async () => {
+  // bugfix for https://sentry.io/organizations/supabase/issues/2903331460/?project=5459134&referrer=slack
+  get.mockImplementation((url) => {
+    if (url.includes('count')) {
+      return {}
+    }
+    return { data: [] }
+  })
+  render(<LogPage />)
+
+  userEvent.click(screen.getByText('Load older'))
+  // NOTE: potential race condition, since we are asserting that something DOES NOT EXIST
+  // wait for 500s to make sure all ui logic is complete
+  // need to wrap in act because internal react state is changing during this time.
+  await act(async () => await wait(100))
+
+  // clicking load older multiple times should not give error
+  await waitFor(() => {
+    expect(screen.queryByText(/Sorry/)).toBeNull()
+    expect(screen.queryByText(/An error occured/)).toBeNull()
+    expect(screen.queryByText(/undefined/)).toBeNull()
+  })
+})