-
Notifications
You must be signed in to change notification settings - Fork 510
fix(storage)!: correct exists() error handling for non-existent files #1745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@mandarini Ya let me look into it. |
|
@itslenny I think this can work if (status && [400, 404].indexOf(status) !== -1) {
return { data: false, error }
}Users checking if (error) will correctly trigger the "not exists" logic when files don't exist The Return Type Supports It Promise<
| { data: boolean; error: null }
| { data: boolean; error: StorageError }
>The change ensures that users can properly handle both "file exists" and "file doesn't exist" scenarios using the error field, which is the standard pattern in Supabase APIs. |
|
I will be parking this PR for now, since it will be a breaking change. We will consider this change for our v3 release. Thank you @SumitKumar-17 |
|
Okay @mandarini |
authored by @SumitKumar-17
Transfered from: supabase/storage-js#256
Bug Fix Report
I have successfully identified and fixed the bug reported in the
issue. Here's what I found and resolved:
The Issue
The bug report was partially correct. The
existsmethod was indeedpresent in both browser and Node.js builds (I confirmed it existed
in the compiled output), but there was an error handling bug that
made it appear to not work properly in certain cases.
The Root Cause
The problem was in the error handling logic of the
existsmethod insrc/packages/StorageFileApi.ts. When a file doesn't exist(resulting in a 404 HTTP status), the method would return
{ data: false, error: StorageError }instead of{ data: false, error: null }.This was incorrect behavior because:
operation failed
{ data: false, error: null }to indicate"file doesn't exist, but the check was successful"
The Fix
I updated the
existsmethod to properly handle bothStorageApiError(which contains the status code directly) andStorageUnknownError(which contains the original error object),and return
{ data: false, error: null }when the error status is400 or 404.
Changes Made
existsmethod to properly return{ data: false, error: null }when a file doesn't existStorageApiErrorexistsmethodVerification
Solves: #1600
Breaking change
This is a breaking change, and it can be added in the v3 planning.