feat(storage): align analytics from method with { data, error } pattern (#1927)

Author: mandarini

package-lock.json

@@ -37,7 +37,7 @@
     "docs:json": "typedoc --json docs/v2/spec.json --entryPoints src/index.ts --entryPoints src/packages/* --excludePrivate --excludeExternals --excludeProtected"
   },
   "dependencies": {
-    "iceberg-js": "^0.8.0",
+    "iceberg-js": "^0.8.1",
     "tslib": "2.8.1"
   },
   "devDependencies": {

packages/core/storage-js/package.json

@@ -1,10 +1,18 @@
-import { IcebergRestCatalog } from 'iceberg-js'
+import { IcebergRestCatalog, IcebergError } from 'iceberg-js'
 import { DEFAULT_HEADERS } from '../lib/constants'
 import { isStorageError, StorageError } from '../lib/errors'
 import { Fetch, get, post, remove } from '../lib/fetch'
 import { isValidBucketName, resolveFetch } from '../lib/helpers'
 import { AnalyticBucket } from '../lib/types'
 
+type WrapAsyncMethod<T> = T extends (...args: infer A) => Promise<infer R>
+  ? (...args: A) => Promise<{ data: R; error: null } | { data: null; error: IcebergError }>
+  : T
+
+export type WrappedIcebergRestCatalog = {
+  [K in keyof IcebergRestCatalog]: WrapAsyncMethod<IcebergRestCatalog[K]>
+}
+
 /**
  * Client class for managing Analytics Buckets using Iceberg tables
  * Provides methods for creating, listing, and deleting analytics buckets
@@ -269,12 +277,14 @@ export default class StorageAnalyticsClient {
    * Get an Iceberg REST Catalog client configured for a specific analytics bucket
    * Use this to perform advanced table and namespace operations within the bucket
    * The returned client provides full access to the Apache Iceberg REST Catalog API
+   * with the Supabase `{ data, error }` pattern for consistent error handling on all operations.
    *
    * **Public alpha:** This API is part of a public alpha release and may not be available to your account type.
    *
    * @category Analytics Buckets
    * @param bucketName - The name of the analytics bucket (warehouse) to connect to
-   * @returns Configured IcebergRestCatalog instance for advanced Iceberg operations
+   * @returns The wrapped Iceberg catalog client
+   * @throws {StorageError} If the bucket name is invalid
    *
    * @example Get catalog and create table
    * ```js
@@ -288,10 +298,10 @@ export default class StorageAnalyticsClient {
    * const catalog = supabase.storage.analytics.from('analytics-data')
    *
    * // Create a namespace
-   * await catalog.createNamespace({ namespace: ['default'] })
+   * const { error: nsError } = await catalog.createNamespace({ namespace: ['default'] })
    *
    * // Create a table with schema
-   * await catalog.createTable(
+   * const { data: tableMetadata, error: tableError } = await catalog.createTable(
    *   { namespace: ['default'] },
    *   {
    *     name: 'events',
@@ -325,7 +335,13 @@ export default class StorageAnalyticsClient {
    * const catalog = supabase.storage.analytics.from('analytics-data')
    *
    * // List all tables in the default namespace
-   * const tables = await catalog.listTables({ namespace: ['default'] })
+   * const { data: tables, error: listError } = await catalog.listTables({ namespace: ['default'] })
+   * if (listError) {
+   *   if (listError.isNotFound()) {
+   *     console.log('Namespace not found')
+   *   }
+   *   return
+   * }
    * console.log(tables) // [{ namespace: ['default'], name: 'events' }]
    * ```
    *
@@ -334,7 +350,7 @@ export default class StorageAnalyticsClient {
    * const catalog = supabase.storage.analytics.from('analytics-data')
    *
    * // List all namespaces
-   * const namespaces = await catalog.listNamespaces()
+   * const { data: namespaces } = await catalog.listNamespaces()
    *
    * // Create namespace with properties
    * await catalog.createNamespace(
@@ -348,57 +364,37 @@ export default class StorageAnalyticsClient {
    * const catalog = supabase.storage.analytics.from('analytics-data')
    *
    * // Drop table with purge option (removes all data)
-   * await catalog.dropTable(
+   * const { error: dropError } = await catalog.dropTable(
    *   { namespace: ['default'], name: 'events' },
    *   { purge: true }
    * )
    *
+   * if (dropError?.isNotFound()) {
+   *   console.log('Table does not exist')
+   * }
+   *
    * // Drop namespace (must be empty)
    * await catalog.dropNamespace({ namespace: ['default'] })
    * ```
    *
-   * @example Error handling with catalog operations
-   * ```js
-   * import { IcebergError } from 'iceberg-js'
-   *
-   * const catalog = supabase.storage.analytics.from('analytics-data')
-   *
-   * try {
-   *   await catalog.dropTable({ namespace: ['default'], name: 'events' }, { purge: true })
-   * } catch (error) {
-   *   // Handle 404 errors (resource not found)
-   *   const is404 =
-   *     (error instanceof IcebergError && error.status === 404) ||
-   *     error?.status === 404 ||
-   *     error?.details?.error?.code === 404
-   *
-   *   if (is404) {
-   *     console.log('Table does not exist')
-   *   } else {
-   *     throw error // Re-throw other errors
-   *   }
-   * }
-   * ```
-   *
    * @remarks
    * This method provides a bridge between Supabase's bucket management and the standard
    * Apache Iceberg REST Catalog API. The bucket name maps to the Iceberg warehouse parameter.
    * All authentication and configuration is handled automatically using your Supabase credentials.
    *
-   * **Error Handling**: Operations may throw `IcebergError` from the iceberg-js library.
-   * Always handle 404 errors gracefully when checking for resource existence.
+   * **Error Handling**: Invalid bucket names throw immediately. All catalog
+   * operations return `{ data, error }` where errors are `IcebergError` instances from iceberg-js.
+   * Use helper methods like `error.isNotFound()` or check `error.status` for specific error handling.
+   * Use `.throwOnError()` on the analytics client if you prefer exceptions for catalog operations.
    *
    * **Cleanup Operations**: When using `dropTable`, the `purge: true` option permanently
    * deletes all table data. Without it, the table is marked as deleted but data remains.
    *
-   * **Library Dependency**: The returned catalog is an instance of `IcebergRestCatalog`
-   * from iceberg-js. For complete API documentation and advanced usage, refer to the
+   * **Library Dependency**: The returned catalog wraps `IcebergRestCatalog` from iceberg-js.
+   * For complete API documentation and advanced usage, refer to the
    * [iceberg-js documentation](https://supabase.github.io/iceberg-js/).
-   *
-   * For advanced Iceberg operations beyond bucket management, you can also install and use
-   * the `iceberg-js` package directly with manual configuration.
    */
-  from(bucketName: string): IcebergRestCatalog {
+  from(bucketName: string): WrappedIcebergRestCatalog {
     // Validate bucket name using same rules as Supabase Storage API backend
     if (!isValidBucketName(bucketName)) {
       throw new StorageError(
@@ -411,7 +407,7 @@ export default class StorageAnalyticsClient {
     // The base URL is /storage/v1/iceberg
     // Note: IcebergRestCatalog from iceberg-js automatically adds /v1/ prefix to API paths
     // so we should NOT append /v1 here (it would cause double /v1/v1/ in the URL)
-    return new IcebergRestCatalog({
+    const catalog = new IcebergRestCatalog({
       baseUrl: this.url,
       catalogName: bucketName, // Maps to the warehouse parameter in Supabase's implementation
       auth: {
@@ -420,5 +416,30 @@ export default class StorageAnalyticsClient {
       },
       fetch: this.fetch,
     })
+
+    const shouldThrowOnError = this.shouldThrowOnError
+
+    const wrappedCatalog = new Proxy(catalog, {
+      get(target, prop: keyof IcebergRestCatalog) {
+        const value = target[prop]
+        if (typeof value !== 'function') {
+          return value
+        }
+
+        return async (...args: unknown[]) => {
+          try {
+            const data = await (value as Function).apply(target, args)
+            return { data, error: null }
+          } catch (error) {
+            if (shouldThrowOnError) {
+              throw error
+            }
+            return { data: null, error: error as IcebergError }
+          }
+        }
+      },
+    }) as unknown as WrappedIcebergRestCatalog
+
+    return wrappedCatalog
   }
 }

packages/core/storage-js/src/packages/StorageAnalyticsClient.ts

@@ -1,40 +1,40 @@
 /**
  * Unit tests for StorageAnalyticsClient.from() method
- * Tests that the method returns a properly configured IcebergRestCatalog instance
+ * Tests that the method returns a wrapped catalog directly and throws on invalid bucket names
  */
 
-import { IcebergRestCatalog } from 'iceberg-js'
 import StorageAnalyticsClient from '../src/packages/StorageAnalyticsClient'
 import { StorageError } from '../src/lib/errors'
 
 describe('StorageAnalyticsClient.from()', () => {
-  it('should return an IcebergRestCatalog instance', () => {
+  it('should return catalog directly for valid bucket name', () => {
     const client = new StorageAnalyticsClient('https://example.supabase.co/storage/v1/iceberg', {
       Authorization: 'Bearer test-token',
     })
 
     const catalog = client.from('my-analytics-bucket')
 
-    expect(catalog).toBeInstanceOf(IcebergRestCatalog)
+    expect(catalog).not.toBeNull()
+    expect(typeof catalog.listNamespaces).toBe('function')
   })
 
-  it('should return different instances for different bucket names', () => {
+  it('should return different catalog instances for different bucket names', () => {
     const client = new StorageAnalyticsClient('https://example.supabase.co/storage/v1/iceberg', {})
 
     const catalog1 = client.from('bucket-1')
     const catalog2 = client.from('bucket-2')
 
-    expect(catalog1).toBeInstanceOf(IcebergRestCatalog)
-    expect(catalog2).toBeInstanceOf(IcebergRestCatalog)
+    expect(catalog1).not.toBeNull()
+    expect(catalog2).not.toBeNull()
     expect(catalog1).not.toBe(catalog2) // Different instances
   })
 
-  it('should work with minimal configuration', () => {
+  it('should return catalog with all expected methods', () => {
     const client = new StorageAnalyticsClient('http://localhost:8181', {})
 
     const catalog = client.from('test-warehouse')
 
-    expect(catalog).toBeInstanceOf(IcebergRestCatalog)
+    expect(catalog).not.toBeNull()
     expect(typeof catalog.listNamespaces).toBe('function')
     expect(typeof catalog.createNamespace).toBe('function')
     expect(typeof catalog.createTable).toBe('function')
@@ -45,12 +45,20 @@ describe('StorageAnalyticsClient.from()', () => {
     expect(typeof catalog.dropNamespace).toBe('function')
   })
 
-  it('should work when called from throwOnError chain', () => {
+  it('should always throw on invalid bucket name (regardless of throwOnError)', () => {
+    const client = new StorageAnalyticsClient('https://example.supabase.co/storage/v1/iceberg', {})
+
+    // Invalid bucket name always throws - it's a programmer error
+    expect(() => client.from('bucket/invalid')).toThrow(StorageError)
+    expect(() => client.throwOnError().from('bucket/invalid')).toThrow(StorageError)
+  })
+
+  it('should return catalog when called from throwOnError chain with valid bucket', () => {
     const client = new StorageAnalyticsClient('https://example.supabase.co/storage/v1/iceberg', {})
 
     const catalog = client.throwOnError().from('my-bucket')
 
-    expect(catalog).toBeInstanceOf(IcebergRestCatalog)
+    expect(catalog).not.toBeNull()
   })
 
   describe('bucket name validation', () => {
@@ -82,38 +90,37 @@ describe('StorageAnalyticsClient.from()', () => {
     })
 
     describe('invalid bucket names', () => {
-      it('should reject empty or null bucket names', () => {
+      it('should throw for empty or null bucket names', () => {
         expect(() => client.from('')).toThrow(StorageError)
         expect(() => client.from(null as any)).toThrow(StorageError)
         expect(() => client.from(undefined as any)).toThrow(StorageError)
       })
 
-      it('should reject path traversal with slashes', () => {
+      it('should throw for path traversal with slashes', () => {
         expect(() => client.from('../etc/passwd')).toThrow(StorageError)
         expect(() => client.from('bucket/../other')).toThrow(StorageError)
-        // Note: '..' alone is valid (just two periods), only with slashes is it path traversal
       })
 
-      it('should reject names with path separators', () => {
+      it('should throw for names with path separators', () => {
         expect(() => client.from('bucket/nested')).toThrow(StorageError)
         expect(() => client.from('/bucket')).toThrow(StorageError)
         expect(() => client.from('bucket/')).toThrow(StorageError)
         expect(() => client.from('bucket\\nested')).toThrow(StorageError)
         expect(() => client.from('path\\to\\bucket')).toThrow(StorageError)
       })
 
-      it('should reject names with leading or trailing whitespace', () => {
+      it('should throw for names with leading or trailing whitespace', () => {
         expect(() => client.from(' bucket')).toThrow(StorageError)
         expect(() => client.from('bucket ')).toThrow(StorageError)
         expect(() => client.from(' bucket ')).toThrow(StorageError)
       })
 
-      it('should reject names exceeding 100 characters', () => {
+      it('should throw for names exceeding 100 characters', () => {
         const tooLongName = 'a'.repeat(101)
         expect(() => client.from(tooLongName)).toThrow(StorageError)
       })
 
-      it('should reject names with unsafe special characters', () => {
+      it('should throw for names with unsafe special characters', () => {
         expect(() => client.from('bucket{name}')).toThrow(StorageError)
         expect(() => client.from('bucket[name]')).toThrow(StorageError)
         expect(() => client.from('bucket<name>')).toThrow(StorageError)
@@ -124,7 +131,7 @@ describe('StorageAnalyticsClient.from()', () => {
       it('should provide clear error messages', () => {
         try {
           client.from('bucket/nested')
-          fail('Should have thrown an error')
+          fail('Expected to throw')
         } catch (error) {
           expect(error).toBeInstanceOf(StorageError)
           expect((error as StorageError).message).toContain('Invalid bucket name')
@@ -134,7 +141,7 @@ describe('StorageAnalyticsClient.from()', () => {
     })
 
     describe('URL encoding behavior', () => {
-      it('should reject strings with percent signs (URL encoding)', () => {
+      it('should throw for strings with percent signs (URL encoding)', () => {
         // The % character is not in the allowed character set, so URL-encoded
         // strings will be rejected. This is correct behavior - users should
         // pass unencoded bucket names.