Skip to content
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

Major DX change: response and error handling #32

Closed
thorwebdev opened this issue Sep 5, 2020 · 15 comments · Fixed by #50
Closed

Major DX change: response and error handling #32

thorwebdev opened this issue Sep 5, 2020 · 15 comments · Fixed by #50
Assignees
Labels
enhancement New feature or request

Comments

@thorwebdev
Copy link
Member

Decision

We've decided to make a major change to support a better developer experience for working with supabase-js. Instead of throwing an error, we will return the error object as part of the response.

Example

Given a successful query, e.g. const response = await supabase.from('products').select('*'), response will be:

{
  "error": null,
  "data": [...],
  "status": 200,
  "statusCode": 200,
  "statusText": "",
  "body": [...] // for backwards compatibility: `body === data`
}

Given a bad query, e.g. const response = await supabase.from('productx').select('*'), response will be:

{
  "error": {
    "code": 404,
    "message": "relation \"public.productx\" does not exist",
    "details": null,
    "hint": null
  },
  "data": null,
  "status": 404,
  "statusCode": 404,
  "statusText": "",
  "body": null // for backwards compatibility: `body === data`
}

Future DX

This will enable the following experience:

const { error, data: products } = await supabase.from('products').select('*')
if (error) {
  alert(error.message)
  return // abort
}
console.log(`Found ${products.length} products.`)

Additional context

Inspiration has been taken from https://github.com/vercel/swr#quick-start:

import useSWR from 'swr'

function Profile() {
  const { data, error } = useSWR('/api/user', fetcher)

  if (error) return <div>failed to load</div>
  if (!data) return <div>loading...</div>
  return <div>hello {data.name}!</div>
}

as well as Stripe.js : https://stripe.com/docs/js/payment_methods/create_payment_method
image

@soedirgo
Copy link
Member

soedirgo commented Sep 6, 2020

Does this error handling include programmer errors? Say I pass a list to match where I expect an object, and I use Object.keys on it. Having to handle each and every kind of such exceptions in JS, with the appropriate error messages, is very painful IMO.

For the record, supabase/postgrest-js#93 uses a fetch polyfill for the HTTP client, so there's no need to use the data field in place of body. Currently the error handling looks more like this, though we can shape it to your proposed format:

const { ok, body: products } = await supabase.from('products').select('*')
if (!ok) { // status code is not 2XX
  alert(products.message) // PostgREST's error is stored in body (`T | T[] | PostgrestError`) in case of error
  return // abort
}
console.log(`Found ${products.length} products.`)

@kiwicopple
Copy link
Member

Does this error handling include programmer errors?

I don't think this should be expected, just catch and rethrow unexpected errors. As you say, handling every possible error would be unwieldy. We should just shape expected errors.

so there's no need to use the data field in place of body.

This one is just a preference. I guess we should decide now what is the more appropriate name for the key. IMO body seems like a throwback to when we used ajax used to request HTML snippets that would be injected into HTML.

Currently the error handling looks more like this

if (!ok) { // status code is not 2XX
  return // abort
}

I'd far prefer this one to look like the suggested change (if (error)). The term ok is unexpected, and error is very clear

soedirgo added a commit to soedirgo/postgrest-js that referenced this issue Sep 15, 2020
@kiwicopple
Copy link
Member

Tasks:

  • Update postgrest-js
  • Update gotrue-js
  • Update realtime-js
  • Update supabase-js
    • Install new libs
    • Change the realtime to work with one socket connection per client

Also solves:

@stupidawesome
Copy link

Are there plans to expose the Content-Range header in the response object?

@soedirgo
Copy link
Member

soedirgo commented Oct 6, 2020

@stupidawesome is there a use case for this? PostgREST supports this, but we haven't really considered having this in. If you just want the COUNT, there's an open issue for it.

kiwicopple added a commit that referenced this issue Nov 2, 2020

- Fixes #32 Major DX change: response and error handling
- Fixes #49 When no `supabaseKey` is passed in it throws an error
- Fixes #31 chore: set up semantic releases
- Fixes #15 `supabase.auth.logout()` throws "Invalid user" error.
- Fixes #20 Auth: Change DX of user management
- Fixes #30 Supabase auth interface missing informiation
- Fixes supabase/supabase#147 supabase/supabase#147
- Partial fix for supabase/realtime-js#53  - if there is no token provided. The error needs to be caught at a socket level.
- Adds magic links


## BREAKING CHANGES

- See all breaking changes in RELEASE.md v1.0.0
- Errors are now returned and not thrown
- Auth now uses `@supabase/gotrue-js` interface
- `supabase.getSubscriptions()` only returns open subscriptions



* Updates the config

* chore: Migrates the basic outline to TS

* Adds a simple example showing how it can be used.

* chore: Moves tests to jest

* chore: Adds semantic releases

* Moves the subscription into it's own class

* Updates the todo readme with simple instructions

* Updates installs

* Revverts commented code - sorry for the spam

* docs: adds JSDoc to some functions

* chore: Adds a function for backwards compat

* chore: migrates the client to SupabaseClient

* This change attempts to make the naming conventions the same as Thor's previously

* Updates GoTrue to latest version

* Adds generic type to the from, and updates the name of the query builder

* Updates to latest versions of all packages

* Updates the example to make sure it's working

* Refactor SupabaseQueryBuilder

* Adds prettier hook

* Add TypeScript next.js example.

* Declutter SupabaseClient and make work with gotrue-js changes.

* Bumps the GoTrue version

* Bumps postgrest to include the types

* Temporarily adds the spec so that I can use it in our docs

* Update examples and add resetPassword.

* Bump gotrue-js version.

* Update lockfile.

* Add auth magic link capabilities.

* Gotrue-js user and session method updates.

* chore: Adds release notes

Co-authored-by: Thorsten Schaeff <thorsten.schaeff@gmail.com>
Co-authored-by: Thor 雷神 Schaeff <5748289+thorwebdev@users.noreply.github.com>
@roker15
Copy link

roker15 commented Mar 8, 2022

const { error, data: products } = await supabase.from('products').select('*')
if (error) {
  alert(error.message)
  return // abort
}
console.log(`Found ${products.length} products.`)

Do i still need to use try-catch after using above pattern?

@soedirgo
Copy link
Member

soedirgo commented Mar 8, 2022

You don't - if it throws it might be sign of a postgrest-js bug.

@roker15
Copy link

roker15 commented Mar 8, 2022

You don't - if it throws it might be sign of a postgrest-js bug.

So does supabase return network failure in error object? I think it does not. So what should i do then?

@soedirgo
Copy link
Member

soedirgo commented Mar 8, 2022

I'm not sure about the other dependencies, but postgrest-js should return network failure in error. If not, can you create an issue in supabase/postgrest-js along with your client lib verison?

@roker15
Copy link

roker15 commented Mar 8, 2022

import useSWR from 'swr'

function Profile() {
  const { data, error } = useSWR('/api/user', fetcher)

  if (error) return <div>failed to load</div>
  if (!data) return <div>loading...</div>
  return <div>hello {data.name}!</div>
}

Here is the text from swr documentation -
**

"If an error is thrown inside fetcher, it will be returned as error by the hook." here is link

**
So if supabase does not throw error and simply return response object how we will catch supabase error inside swr?

Will following code work then? Because in following code supabase is not throwing error, so what will be error in following code?

const { data, error } = useSWR(
    subheadingId == undefined || userid == undefined
      ? null
      : [`/subheading/${subheadingId}/${userid}`],
    async () =>
      await supabase.rpc<SharedNotesList>("subheadingg", {
        subheadingid: subheadingId,
        sharedwith: userid,
      }),
     );

@soedirgo
Copy link
Member

soedirgo commented Mar 8, 2022

If you want the .rpc() call to throw errors, you can use .throwOnError() (example: https://github.com/supabase/postgrest-js/blob/a972b993487fe99990a6be7995f0c6a2dc50b784/test/basic.ts#L136)

@roker15
Copy link

roker15 commented Mar 8, 2022

If you want the .rpc() call to throw errors, you can use .throwOnError() (example: https://github.com/supabase/postgrest-js/blob/a972b993487fe99990a6be7995f0c6a2dc50b784/test/basic.ts#L136)

I am not specific about rpc. take it as general supabase query and answer what i am asking please.

@soedirgo
Copy link
Member

soedirgo commented Mar 8, 2022

This is not specific to .rpc(). You can use .throwOnError() on .select(), .insert(), etc. and it will throw errors instead of returning it in the response object. See the example I linked above.

@n-sviridenko
Copy link

n-sviridenko commented Jun 15, 2022

Can we set this as a default behaviour globally via a parameter or smth? This is super unusual behaviour, as SDKs ussually throw errors, otherwise we have to add lots of additional checks around that which doesn't make sense.

@rienheuver
Copy link

Throwing errors by default, or have that as an option on createClient, would be a great improvement for us too. We use a lot of rxjs with supabase and regular error throwing would make the whole process more logical. Now we have to remember to write throwOnError after every query, because otherwise the error will go silent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment