feat: throw on error types

[mmkal]

What kind of change does this PR introduce?

Re https://github.com/supabase/supabase-js/issues/885 and https://github.com/supabase/supabase-js/issues/801 (would likely need a small change to supabase-js too once this is published, to close them)

What is the current behavior?

1. throwOnError() has no effect at compile-time
2. There's no way to set a client to always throw on errors

See issues above.

What is the new behavior?

The return type of a .throwOnError()'d query is always a "success" response at compile time. i.e. error is always null and data isn't unioned with null (though it can still be null for .maybeSingle() etc.)

Breaking change (albeit fairly small one): .throwOnError() now has to be either at the "start" or the "end" of a chain. This is no longer allowed:

await postgrest.from('users').select('*').throwOnError().single()

It would have to be either:

await postgrest.from('users').select('*').single().throwOnError()

or the client can be configured to have all queries throw on error:

const strictClient = postgres.throwOnError()
await strictClient.from('users').select('*').single()

Additional context

The motivator for me wanting this is the ugliness of littering supabase codebases with go-like err != nil checks:

const {data, error} = await db.from('users').select('*').eq('id', foo).single()

if (error) {
  throw error
}

console.log(`Hello ${data.username}`)

It gets even worse if you need multiple queries in the same scope. It's becomes awkward to alias data to anything meaningful, because then you also need to

const {data: user, error: userError} = await db.from('users').select('*').eq('id', foo).single()

if (userError) {
  throw userError
}

console.log(`Hello ${data.username}`)

const {data: orders, error: ordersError} = await db.from('orders').select('*').eq('user_id', user.id)

if (ordersError) {
  throw ordersError
}

You also need to avoid lint rules that get angry at throwing a "raw" value without re-wrapping.

This would allow doing export const createDBClient = () => createClient(...).throwOnError() in a single helper, then all queries would have the throwy behaviour. The above example would become:

const {data: user} = await db.from('users').select('*').eq('id', foo).single()

console.log(`Hello ${data.username}`)

const {data: orders} = await db.from('orders').select('*').eq('user_id', user.id)

Implementation

It ended up being a bigger change than I'd hoped, because the ThrowOnError typearg needed to be passed around everywhere. The breaking change was needed because the old throwOnError() implementation had a return type of this - and as far as I know there's no such concept in TypeScript as this but with typearg ThrowOnError = true. So, an abstract class can't have a method that returns a variant of its implementer - meaning the throwOnError method on PostgrestBuilder needs to return a PostgrestBuilder, not the concrete implementation via this. So it knows about the Result type but not about the rest of the fanciness on PostgrestTransformBuilder.

It might be possible to keep the old put-throwOnError-anywhere behaviour, but it would require quite a bit more fiddling and this seemed like a big enough change for me on a repo I'm not familiar with.

[mmkal]

CC @soedirgo @steve-chavez @wyozi any thoughts on this?

---

Copying my comment on https://github.com/supabase/supabase-js/issues/801

I got impatient so I published it under @rebundled/postgrest-js. Here's how you can use it with @supabase/ssr:

npm install @rebundled/postgrest-js

import { CookieOptions, createServerClient } from '@supabase/ssr'
import { PostgrestClient } from '@rebundled/postgrest-js'
import { cookies } from 'next/headers'
import { Database } from '~/db'

export const createClient = () => {
  const client = createServerClient<Database>(
    process.env.NEXT_PUBLIC_SUPABASE_URL!,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
    {
      cookies: {
        get(name: string) {
          return cookieStore.get(name)?.value
        },
      },
    },
  )

  // @ts-expect-error `.rest` is protected
  const { rest } = client
  const wrapper = new PostgrestClient<Database>(rest.url, {
    fetch: rest.fetch,
    headers: rest.headers,
    schema: rest.schemaName,
  }).throwOnError()
  return Object.assign(wrapper, {
    rest,
    realtime: client.realtime,
    auth: client.auth,
  })
}

You can then use like:

import {createClient} from '..'

export default async () => {
  const client = createClient()

  const { data: user } = await client.from('users').select('*').single()

  console.log(`Hello ${user.username}`) // no need to check truthiness/ throw manually
}

If you need access to the non-throwing client, you can use createClient().rest. Note that if the PR is merged, it'll probably make sense to make a change in the supabase-js too, so that explicitly creating a new PostgrestClient wouldn't be necessary. But this seemed like a non-invasive way to let people try this out now.

[wyozi]

It seems like a good change that I would find useful, but then again I'm a mere contributor :) Seems like Supabase team is not responding to PRs very quickly at the moment, which is the reason why there's e.g. 3 separate spread operator PRs open at the moment.

[AlbertMarashi]

+1 on getting this reviewed & merged plz

[mmkal]

Tagging some more people who seem to have been reviewing PRs and commiting code etc., indicating to me this project is not abandoned: @soedirgo @abhadu

This PR only really affects types, and adds new type tests to make sure there aren't new regressions and that the types work correctly. It's also a big quality-of-life improvement so I think it's worth taking a look!

I've updated from master, but it was fairly painful resolving the merge conflicts. This was work that would not have existed if this had gone in before @soedirgo's change https://github.com/supabase/postgrest-js/commit/f91aa2944f52da3aefc77a6712990731fff16252 - and I would rather not have to keep resolving merge conflicts if all that needs to happen is for this to be reviewed!