supabase / storage-js

JS Client library to interact with Supabase Storage
Apache License 2.0
131 stars 40 forks source link

StorageError shape doesn't match returned errors from API #165

Open nlarusstone opened 1 year ago

nlarusstone commented 1 year ago

Bug report

Describe the bug

The StorageApiError class defined in storage-js has a status field. The StorageError defined in storage-api has a statusCode field.

This mismatch make it difficult to correctly handle errors on the frontend. I'd like to use the isStorageError function defined in storage-js, but it does not recognize errors thrown from the API

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Upload a file
  2. Upload the same file without allowing upserts
  3. Throw the returned error
  const { error } = await supabase.storage
    .from('bucket')
    .upload(filename, buffer, {
      upsert: false,
    });
  console.log(isStorageError(error)) -- this is false, but should be true

Expected behavior

isStorageError correctly identifies when the storage API has thrown an error. StorageApiError has the correct fields that correspond to the returned error.

System information

bombillazo commented 1 year ago

Any update on this?

dogukanakkaya commented 1 year ago

Any update here?

I inspected the codebase a bit and seems like some places throw StorageError and some places are returning the json response returned by the api which has (should have) these type definition:

interface StorageResponseError {
  statusCode: string; // I'd also propose to return an integer type from the api instead of string
  error: string;
  message: string;
}

I didn't wanted to touch the responses since I don't know the internal errors thrown from the api and don't have time to deep dive. But some methods like upload just needs to return this above type instead. We need a "single error type to rule them all". Error class has extra properties which are not returned in api errors like stack. So type must be defined instead of defining StorageError as return type.

Raepheles commented 2 months ago

Is there going to be an update on this? More than a year since this issue is created and StorageError still doesn't match the object we get from the API.