Open ianschmitz opened 7 months ago
Fully agree, the proposal for setting this as default or as an option on createClient
got upvoted a couple times in #32. Please do consider this supabase team 😄
Actually just came to create this issue, then saw it here.
Storage doesn't even have this as option yet https://github.com/supabase/storage-js/issues/150.
It'd be a breaking change to make this default, so this should just be an option when creating a client. Just throwOnError: true
would be fantastic.
Plus, if this feature is enabled globally, any call should automatically return response.data
instead of the encapsulated { data }
object. Right now, my code is littered with .throwOnError().then(res => res.data)
for every call to Supabase.
Many new Supabase users make the mistake of not handling errors from the sdk correctly, and end up wasting time figuring out issues because of that. Giving a throwOnError: true
option would be a sane default.
Completely agree @ConProgramming. Thanks for the suggestions!
Alternatively, let us set throwOnErrors
when creating the client. Most users are/should be using a wrapper anyway:
import { createClient } from '@supabase/supabase-js'
export const createSupabase = async () => {
return createClient('https://xyzcompany.supabase.co', 'public-anon-key', {
throwOnErrors: true
})
}
This, along with a fix for #801, would cut out loads of boilerplate. @kiwicopple @w3b6x9 @filipecabaco would you accept a PR for this?
I wholeheartedly support this proposal. I would go so far as to say that going with the result pattern by default was a bold design choice but also a highly questionable one. Throwing on errors should be the default in any library. Returning result objects is useful in certain scenarios but should be implemented in the application instead of in a library. For our project it is cumbersome to integrate with supabase for the reasons mentioned in this thread.
Downsides of the Result pattern:
throwOnError
).data
and error
need to be renamed for every request.There is nothin wrong with having an option for using result objects instead of standard error handling, but it should be opt-in and not the default.
I'd like to ask you to at least consider changing this in the next major release.
would you accept a PR for this?
The first step would be to get throwOnError
inside every client lib (this is somethign that we want, so we will definitely accept PRs)
The next step, making it the default, is a breaking change - so it needs more discussion. Either way, step 1 seems like good progress so that we can have a throwOnError
on supabase-js
which propogates down to every other lib
would you accept a PR for this?
The first step would be to get
throwOnError
inside every client lib (this is somethign that we want, so we will definitely accept PRs)The next step, making it the default, is a breaking change - so it needs more discussion. Either way, step 1 seems like good progress so that we can have a
throwOnError
onsupabase-js
which propogates down to every other lib
@kiwicopple have you seen the PR I opened? https://github.com/supabase/postgrest-js/pull/494
Bug report
Describe the bug
Admittedly this is not a bug, but i wanted this issue to end up in the
supabase-js
repo as that is where the decision would need to be made.As a new user of the
supabase-js
library, i was caught off guard by the API not throwing errors by default. Instead you need to remember to invokethrowOnError()
after every query. This change was previously made and discussed here: https://github.com/supabase/supabase-js/issues/32, and it seems others share my same concern.This is (IMO) an unusual behavior for an API where errors are silent by default.
.update()
mutations that fail can easily go unnoticed for example.Other popular libraries i use on a regular basis such as Prisma, Zod, etc. instead throw by default, and you either have to catch the error, or opt into the API that doesn't throw and returns an error object (see Zod docs for example).
cc @n-sviridenko, @rienheuver