Proposal: throwOnError should be default behavior

[ianschmitz]

Bug report

• I confirm this is a bug with Supabase, not with my own application.
• I confirm I have searched the Docs, GitHub Discussions, and Discord.

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 invoke throwOnError() after every query. This change was previously made and discussed here: #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

[rienheuver]

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 😄

[ConProgramming]

Actually just came to create this issue, then saw it here.

Storage doesn't even have this as option yet supabase/storage-js#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.

[ianschmitz]

Completely agree @ConProgramming. Thanks for the suggestions!

[mmkal]

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?

[juni0r]

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:

• There is no universal convention on how a result object is suppposed to look like, especially the error object.
• You either have to opt into this pattern for the rest of the application or employ nasty workarounds to get the usual throwing-behavior (throwOnError).
• Libraries should not make design choices on that level. Rather, employ the standard error handling behavior and have consumers of the library wrap the API calls if they want to use the result pattern.
• Constructs like Promise.all don't work with the result pattern.
• It interferes with certain centralized error-handling strategies.
• The client code gets nasty as soon as you make more than one request in a single scope, as 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.

[kiwicopple]

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

[mmkal]

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

@kiwicopple have you seen the PR I opened?
supabase/postgrest-js#494

[gurumaxi]

Any update on this one?

[its-nmt05]

Any update on this?
Still having to use throwOnError() this after almost a year

[gurumaxi]

Same here, this would reduce boilerplate code a lot in my project

[Viriatto]

Bumping this aswell, I've also noticed that when chaining throwOnError() with single() the resulting data's inferred type union still includes null even though it should never be null (I think?)

[duncannz]

@Viriatto That sounds like #801, which was just resolved in the latest version.

(Regardless, I still strongly agree with the OP, the current behaviour requires cumbersome boilerplate to handle errors which is also easy to forget)

Edit: It turns out that although the fix for #801 works when using await, it still incorrectly adds null to the return type when using .then(), as per supabase/postgrest-js#601.