add `unwrap()` method to supabase client

[andykais]

Feature request

add an unwrap() method or a an either monad left() method to pull the data out of a postgrest response.

Is your feature request related to a problem? Please describe.

Not exactly a problem, but the way the client is designed requires every database query to individually handle database failures. For the most part, I want to assume that the query succeeded, and throw an error if it did not. That makes most of my db calls look like this:

  const res = await context.supabase
    .from<tables['my_table']>('my_table')
    .select()
    .filter('status', 'eq', 'COMPLETED')
    .limit(1)
  if (res.error) throw new Error(`SupabaseError: query failed: \n${res.error}`)

Describe the solution you'd like

The solution I would like best, would be to add a method which can optionally unwrap the { error: object } | { data: object[] } response from supabase into just the data field. If there is an error present, the client should throw an error. This should not break any existing workflows, its just an optional convenience method.

Usage:

const data: table['my_table'] = await supabase.from<table['my_table']>('my_table')
  .select()
  .unwrap()

// handling an error:
try {
  const data = await supabase.from('my_table').delete().unwrap()
} catch (e) {
  if (e.name === 'SupabaseError') {
    // handle it
  } else {
    throw e
  }
}

Describe alternatives you've considered

I have attempted building this on top of the supabase client, but because of the way queries are built, I need to use proxies:

  const supabase_proxy_handler = {
    get: function (target: any, prop: string) {
      if (prop === 'unwrap') {
        return new Promise((resolve, reject) => {
          target.then((res: PostgrestResponse) => {
            if (res.error) throw new Error(`SupabaseError: query failed: \n${JSON.stringify(res.error)}`)
            else return res.data
          })
        })
      } else {
        return target[prop]
      }
    },
  }
  const supabase_proxy = new Proxy(supabase, supabase_proxy_handler)

This technically works, but manipulating the types to include the unwrap() method is harder.

Additional context

unwrap is a common pattern in several languages & libraries. Here is an example in rust: https://doc.rust-lang.org/rust-by-example/error/option_unwrap.html

[emschwartz]

Out of curiosity, why do all the calls return an error instead of rejecting the Promise? This seems like a very unusual pattern for Javascript/Typescript.

[andykais]

@emschwartz they are rejecting the promise. This is async/await syntax, which can use try/catch blocks to catch rejected promises

[emschwartz]

They are? I'm familiar with async/await syntax but all of the examples here show data and error in the object that the Promise resolves to:

const { data, error } = await supabase
  .from('cities')

As as result, it seems like we need to write code like this:

try {
  const { data, error } = await supabase
    .from('cities')
  if (error) {
    throw error
  }
} catch (err) {
  // handle err
}

If the Promise were rejected instead of returning the error object, you could leave out the if (error) { throw error } bit.

(This is the line I would expect to throw instead of returning)

[andykais]

I understand your confusion now. If you look at my first message, the error is being thrown in the "Describe the solution you'd like" section. This is my suggestion on how an unwrap method should work. Above that I have an example of how the supabase client works currently, which is what you are describing.

[emschwartz]

Oh whoops, @andykais sorry for the misunderstanding. My question was actually about the library as it currently stands rather than your suggestion.

I think your proposal seems like a reasonable way to improve the ergonomics without breaking backwards compatibility.

If there is going to be a major version change with breaking changes, I would vote for the pattern to be changed entirely so that all errors cause the Promise to be rejected.

[kiwicopple]

Hi @emschwartz - this is just a DX decision. We purposely chose to return errors rather than throw them. There is no right or wrong here, only preferences. We decided to return because (we believe) it is a nicer developer experience :) . I'm sorry you don't share the view - I didn't either until I started using it, so perhaps this library will change your mind

[emschwartz]

Hi @kiwicopple I hear that and could accept that it's just a different style choice if the Promise were guaranteed never to reject (because then you wouldn't need to wrap it in a try/catch). However, this call to fetch can "reject on network failure or if anything prevented the request from completing" (MDN), which means you need to handle the error in two places instead of one. Always wrapping it in a try/catch and re-throwing the error returned seems worse than handling all types of errors in one way.

All of that said, I'm using your open source library so take my comments with a grain of salt :)

[malobre]

I agree with @emschwartz, we end up try-catch'ing + checking for error in the returned object.
If the goal was to enhance the DX I think it should be harmonized (e.g catch thrown errors and put them in res.error).

Edit: The auth part already does this (e.g signUp)

[Nick-Mazuk]

I do second this feature request.

Since a database call isn't usually the only thing that's happening, I've found that I've often needed to double catch errors. For instance:

try {
    const { data, error } = await supabase.from('table').select('*')
    if (error) throw new Error(error)

    // do other post-processing
} catch (error) {
    // handle the errors
}

This is a short example, but it can get especially cumbersome when combining this with other API calls for other services. Here's a contrived example that should hopefully get the point across.

try {
    const [{ data, error }, fetchResponse, otherApiResponse] = await Promise.all([
        supabase.from('table').select('*'),
        fetch('https://example.com/api-endpoint'),
        someOtherApiFunctionCall(),
    ])
    if (error) throw new Error(error)

    // do other post-processing
} catch (error) {
    // handle the errors
}

Notice how the Supabase error is handled differently from everything else?

Or what if you don't use async/await, and you use then/catch. For instance, inside of a react useEffect hook, then/catch is preferred since you cannot use async/await directly in hook.

const [posts, setPosts] = useState()
const [errorMessage, setErrorMessage] = useState('')

// example with no error handling
useEffect(() => {
    supabase
        .from('posts')
        .select('*')
        .then(({ data }) => setPosts(data))
}, [])

// with error handling
useEffect(() => {
    const getData = async () => {
        const { data, error } = await supabase.from('posts').select('*')
        if (error) setError(error)
        else setPosts(data)
    }
    getData()
}, [])

// error handling if supabase unwrapped promises
useEffect(() => {
    supabase
        .from('posts')
        .select('*')
        .unwrap()
        .then((data) => setPosts(data))
        .catch((error) => setError(error))
}, [])

I would argue that the unwrap option is the clearest option to read and write.

I understand that this is a matter of opinion and not objective fact, but when a lot of JavaScript is built around being able to reject promises, I find it hard to believe that not rejecting on errors is always better. At least having the option with unwrap I think is much better than the current schema.

[mstade]

For what it's worth, the decision to return the error as a result rather than throw is described in #32. I opened a discussion about this some time ago asking pretty much the same thing as you, with a slightly (but not meaningfully) different take on it: supabase/supabase#604

We've found that we use both the { data, error } = supabase ... pattern and a variant of unwrap, so I'm not sure either way is strictly better and an .unwrap() modifier seems like a good way to go. What we do with our variant right now is (ironically) to wrap the query and throw if there's an error, like so:

try { 
  const data = await throwOnError(supabase ...)
} catch (error) {
  ...
}

I'm gonna take a stab at a PR for this, because it's annoying me to no end. 😅

[mstade]

Proposed implementation available in supabase/postgrest-js#188.