fix: handle readonly bucket states safely (#188)

Author: cxymds

app/(dashboard)/browser/content.tsx

@@ -18,6 +18,7 @@ import { useTasks } from "@/contexts/task-context"
 import { ObjectPreviewModal } from "@/components/object/preview-modal"
 import { useObject } from "@/hooks/use-object"
 import { usePermissions } from "@/hooks/use-permissions"
+import { isMissingBucketError } from "@/lib/bucket-access"
 
 interface BrowserContentProps {
   bucketName: string
@@ -51,15 +52,9 @@ export function BrowserContent({ bucketName, keyPath = "", preview = false, prev
     headBucket(bucketName)
       .then(() => {})
       .catch((error: unknown) => {
-        const err = error as { $metadata?: { httpStatusCode?: number }; Code?: string; message?: string }
-        const status = err?.$metadata?.httpStatusCode
-        const code = (err?.Code ?? (error as Error)?.message ?? "").toLowerCase()
-        const isAccessDenied =
-          status === 403 ||
-          code === "accessdenied" ||
-          code === "forbidden" ||
-          (typeof code === "string" && (code.includes("access denied") || code.includes("forbidden")))
-        message.error(isAccessDenied ? t("Access Denied") : t("Bucket not found"))
+        if (!isMissingBucketError(error)) return
+
+        message.error(t("Bucket not found"))
         const params = new URLSearchParams(searchParams.toString())
         params.delete("bucket")
         params.delete("prefix")

app/(dashboard)/browser/page.tsx

@@ -21,6 +21,7 @@ import { useMessage } from "@/lib/feedback/message"
 import { formatDateTime, formatInteger, niceBytes } from "@/lib/functions"
 import { normalizeDateToIso } from "@/lib/safe-date"
 import { getAccountBucketUsage } from "@/lib/account-bucket-usage"
+import { loadBucketPolicyStatuses } from "@/lib/bucket-policy-status"
 import { BrowserContent } from "./content"
 import type { ColumnDef } from "@tanstack/react-table"
 
@@ -58,26 +59,13 @@ function BrowserBucketsPage() {
       }
 
       try {
-        const results = await Promise.all(
-          bucketNames.map(async (name) => {
-            try {
-              const resp = (await getBucketPolicyStatus(name)) as {
-                PolicyStatus?: { IsPublic?: boolean }
-              }
-              return { name, isPublic: resp?.PolicyStatus?.IsPublic === true }
-            } catch {
-              return { name, isPublic: false }
-            }
-          }),
-        )
+        const policyMap = await loadBucketPolicyStatuses(bucketNames, getBucketPolicyStatus)
         if (fetchId !== fetchIdRef.current) return
 
-        const policyMap = Object.fromEntries(results.map((r) => [r.name, r.isPublic]))
-
         setData((prev) =>
           prev.map((row) => ({
             ...row,
-            IsPublic: policyMap[row.Name] ?? false,
+            IsPublic: policyMap[row.Name],
           })),
         )
       } catch {
@@ -226,7 +214,7 @@ function BrowserBucketsPage() {
           <span className="text-muted-foreground">{t("Private")}</span>
         )
       }
-      return policyLoading ? <Spinner className="size-3 text-muted-foreground" /> : "--"
+      return policyLoading ? <Spinner className="size-3 text-muted-foreground" /> : "-"
     },
   })
 

components/buckets/list.tsx

@@ -14,6 +14,7 @@ import { Spinner } from "@/components/ui/spinner"
 import { formatDateTime, formatInteger, niceBytes } from "@/lib/functions"
 import { normalizeDateToIso } from "@/lib/safe-date"
 import { getAccountBucketUsage } from "@/lib/account-bucket-usage"
+import { loadBucketPolicyStatuses } from "@/lib/bucket-policy-status"
 import type { ColumnDef } from "@tanstack/react-table"
 
 export interface BucketListRow {
@@ -50,25 +51,13 @@ export function BucketList({ title, emptyDescription, getBucketHref }: BucketLis
       }
 
       try {
-        const results = await Promise.all(
-          bucketNames.map(async (name) => {
-            try {
-              const resp = (await getBucketPolicyStatus(name)) as {
-                PolicyStatus?: { IsPublic?: boolean }
-              }
-              return { name, isPublic: resp?.PolicyStatus?.IsPublic === true }
-            } catch {
-              return { name, isPublic: false }
-            }
-          }),
-        )
+        const policyMap = await loadBucketPolicyStatuses(bucketNames, getBucketPolicyStatus)
         if (fetchId !== fetchIdRef.current) return
 
-        const policyMap = Object.fromEntries(results.map((r) => [r.name, r.isPublic]))
         setData((prev) =>
           prev.map((row) => ({
             ...row,
-            IsPublic: policyMap[row.Name] ?? false,
+            IsPublic: policyMap[row.Name],
           })),
         )
       } finally {
@@ -202,7 +191,7 @@ export function BucketList({ title, emptyDescription, getBucketHref }: BucketLis
               <span className="text-muted-foreground">{t("Private")}</span>
             )
           }
-          return policyLoading ? <Spinner className="size-3 text-muted-foreground" /> : "--"
+          return policyLoading ? <Spinner className="size-3 text-muted-foreground" /> : "-"
         },
       },
     ],

components/data-table/data-table.tsx

@@ -12,6 +12,8 @@ import { cn } from "@/lib/utils"
 interface DataTableProps<TData> {
   table: Table<TData>
   isLoading?: boolean
+  errorTitle?: string
+  errorDescription?: string
   emptyTitle?: string
   emptyDescription?: string
   caption?: string
@@ -67,6 +69,8 @@ function getAriaSort<TData>(column: Column<TData, unknown>): React.AriaAttribute
 export function DataTable<TData>({
   table,
   isLoading = false,
+  errorTitle,
+  errorDescription,
   emptyTitle = "No data",
   emptyDescription = "There is nothing to display yet.",
   caption,
@@ -132,6 +136,14 @@ export function DataTable<TData>({
               </div>
             </TableCell>
           </TableRow>
+        ) : errorTitle ? (
+          <TableRow>
+            <TableCell colSpan={visibleColumnCount} className="h-48">
+              <div role="alert">
+                <EmptyState title={errorTitle} description={errorDescription} />
+              </div>
+            </TableCell>
+          </TableRow>
         ) : hasRows ? (
           table.getRowModel().rows.map((row) => (
             <TableRow

components/object/list.tsx

@@ -87,6 +87,21 @@ interface ObjectRow {
   LastModified: string
 }
 
+type ObjectListError = "accessDenied" | "loadFailed"
+
+function isAccessDeniedError(error: unknown): boolean {
+  const serviceError = error as {
+    $metadata?: { httpStatusCode?: number }
+    Code?: string
+    name?: string
+    message?: string
+  }
+  if (serviceError?.$metadata?.httpStatusCode === 403) return true
+
+  const code = (serviceError?.Code ?? serviceError?.name ?? serviceError?.message ?? "").toLowerCase()
+  return code === "accessdenied" || code === "forbidden" || code.includes("access denied")
+}
+
 interface ObjectListProps {
   bucket: string
   path: string
@@ -123,12 +138,11 @@ export function ObjectList({
   const [searchTerm, setSearchTerm] = React.useState("")
   const [showDeleted, setShowDeleted] = useLocalStorage("object-list-show-deleted", false)
   const [loading, setLoading] = React.useState(false)
+  const [listError, setListError] = React.useState<ObjectListError | null>(null)
   const [data, setData] = React.useState<ObjectRow[]>([])
   const [nextToken, setNextToken] = React.useState<string | undefined>()
   const [showScrollShortcuts, setShowScrollShortcuts] = React.useState(false)
   const [bucketVersioningState, setBucketVersioningState] = React.useState<BucketVersioningState>("unknown")
-  const [versioningError, setVersioningError] = React.useState("")
-  const [versioningReload, setVersioningReload] = React.useState(0)
   const [deleteDialogOpen, setDeleteDialogOpen] = React.useState(false)
   const [deleteDialogKeys, setDeleteDialogKeys] = React.useState<string[]>([])
   const [deleteAllVersions, setDeleteAllVersions] = React.useState(false)
@@ -203,6 +217,7 @@ export function ObjectList({
       const requestScope = activeScopeRef.current
       loadingRef.current = true
       setLoading(true)
+      if (!shouldAppend) setListError(null)
       try {
         const response = await listObject(bucket, prefix || undefined, resolvedPageSize, token, {
           includeDeleted: showDeleted,
@@ -226,6 +241,7 @@ export function ObjectList({
         }
 
         setNextToken(r.NextContinuationToken)
+        setListError(null)
 
         const prefixItems: ObjectRow[] = (r.CommonPrefixes ?? []).map((item) => ({
           Key: item.Prefix ?? "",
@@ -261,6 +277,7 @@ export function ObjectList({
           setNextToken(undefined)
           if (!shouldAppend) {
             setData([])
+            setListError(isAccessDeniedError(error) ? "accessDenied" : "loadFailed")
           }
         }
       } finally {
@@ -327,7 +344,6 @@ export function ObjectList({
 
     const loadBucketVersioningStatus = async () => {
       setBucketVersioningState("unknown")
-      setVersioningError("")
 
       try {
         const resp = await getBucketVersioning(bucket)
@@ -338,7 +354,6 @@ export function ObjectList({
         console.error("Failed to load bucket versioning status:", error)
         if (!cancelled) {
           setBucketVersioningState("unknown")
-          setVersioningError(t("Failed to get data"))
         }
       }
     }
@@ -348,7 +363,7 @@ export function ObjectList({
     return () => {
       cancelled = true
     }
-  }, [bucket, getBucketVersioning, t, versioningReload])
+  }, [bucket, getBucketVersioning])
 
   const displayKey = React.useCallback(
     (key: string) => {
@@ -531,7 +546,6 @@ export function ObjectList({
                 size="sm"
                 onClick={() => openDeleteDialog([row.original.Key])}
                 disabled={bucketVersioningState === "unknown"}
-                aria-describedby={bucketVersioningState === "unknown" ? "object-versioning-status" : undefined}
               >
                 <RiDeleteBin5Line className="size-4" aria-hidden />
                 <span>{t("Delete")}</span>
@@ -781,7 +795,6 @@ export function ObjectList({
                     className="border-destructive text-destructive"
                     onClick={handleBatchDelete}
                     disabled={bucketVersioningState === "unknown"}
-                    aria-describedby={bucketVersioningState === "unknown" ? "object-versioning-status" : undefined}
                   >
                     <RiDeleteBin5Line className="size-4" aria-hidden />
                     <span>{t("Delete Selected")}</span>
@@ -824,44 +837,30 @@ export function ObjectList({
         </div>
       </PageHeader>
 
-      {bucketVersioningState === "unknown" ? (
-        <div
-          id="object-versioning-status"
-          role={versioningError ? "alert" : "status"}
-          className="flex flex-col gap-3 border border-border bg-muted/30 p-3 text-sm sm:flex-row sm:items-center sm:justify-between"
-        >
-          <span className={versioningError ? "text-destructive" : "text-muted-foreground"}>
-            {versioningError || t("Loading…")}
-          </span>
-          {versioningError ? (
-            <Button
-              type="button"
-              variant="outline"
-              className="w-full sm:w-auto"
-              onClick={() => setVersioningReload((value) => value + 1)}
-            >
-              <RiRefreshLine className="size-4" aria-hidden />
-              {t("Refresh")}
-            </Button>
-          ) : null}
-        </div>
-      ) : null}
-
       <DataTable
         table={table}
         isLoading={displayState === "loading" || displayState === "filtered-loading"}
+        errorTitle={
+          listError === "accessDenied"
+            ? t("Access Denied")
+            : listError === "loadFailed"
+              ? t("Failed to load objects")
+              : undefined
+        }
         emptyTitle={emptyTitle}
         emptyDescription={emptyDescription}
       />
 
-      <div className="flex flex-wrap items-center justify-between gap-3 text-sm text-muted-foreground">
-        <span>
-          {t("Loaded {count} objects", {
-            count: data.length,
-          })}
-        </span>
-        <span>{t("Filtering and sorting apply to loaded objects")}</span>
-      </div>
+      {!listError ? (
+        <div className="flex flex-wrap items-center justify-between gap-3 text-sm text-muted-foreground">
+          <span>
+            {t("Loaded {count} objects", {
+              count: data.length,
+            })}
+          </span>
+          <span>{t("Filtering and sorting apply to loaded objects")}</span>
+        </div>
+      ) : null}
 
       {nextToken ? (
         <div ref={loadMoreRef} className="flex min-h-10 items-center justify-center text-sm text-muted-foreground">

lib/bucket-access.ts

@@ -0,0 +1,16 @@
+interface S3ServiceError {
+  $metadata?: {
+    httpStatusCode?: number
+  }
+  Code?: string
+  name?: string
+  message?: string
+}
+
+export function isMissingBucketError(error: unknown): boolean {
+  const serviceError = error as S3ServiceError
+  if (serviceError?.$metadata?.httpStatusCode === 404) return true
+
+  const code = (serviceError?.Code ?? serviceError?.name ?? "").toLowerCase()
+  return code === "nosuchbucket" || code === "notfound"
+}

lib/bucket-policy-status.ts

@@ -0,0 +1,23 @@
+export interface BucketPolicyStatusResponse {
+  PolicyStatus?: {
+    IsPublic?: boolean
+  }
+}
+
+export async function loadBucketPolicyStatuses(
+  bucketNames: string[],
+  getBucketPolicyStatus: (bucketName: string) => Promise<unknown>,
+): Promise<Record<string, boolean | undefined>> {
+  const results = await Promise.all(
+    bucketNames.map(async (name) => {
+      try {
+        const response = (await getBucketPolicyStatus(name)) as BucketPolicyStatusResponse
+        return [name, response.PolicyStatus?.IsPublic] as const
+      } catch {
+        return [name, undefined] as const
+      }
+    }),
+  )
+
+  return Object.fromEntries(results)
+}

tests/lib/bucket-access.test.ts

@@ -0,0 +1,17 @@
+import test from "node:test"
+import assert from "node:assert/strict"
+
+import { isMissingBucketError } from "../../lib/bucket-access"
+
+test("isMissingBucketError redirects only for a definitive missing bucket response", () => {
+  assert.equal(isMissingBucketError({ $metadata: { httpStatusCode: 404 } }), true)
+  assert.equal(isMissingBucketError({ Code: "NoSuchBucket" }), true)
+})
+
+test("isMissingBucketError does not treat access denied as a missing bucket", () => {
+  assert.equal(isMissingBucketError({ $metadata: { httpStatusCode: 403 }, Code: "AccessDenied" }), false)
+})
+
+test("isMissingBucketError keeps the current bucket when availability is uncertain", () => {
+  assert.equal(isMissingBucketError(new Error("Network request failed")), false)
+})