From 9df1e84750a2d83552d540e711c345b00f3ec1b3 Mon Sep 17 00:00:00 2001
From: Marcus Stade <mstade@users.noreply.github.com>
Date: Fri, 11 Jun 2021 18:48:38 +0200
Subject: [PATCH] feat: Implement unwrap() to throw query errors instead of
 returning them (#188)

* Implement unwrap() to throw query errors instead of returning them

The rationale and motivation for this change is well described in supabase/supabase-js#92,
and further supported by the discussion in supabase/supabase#604.

This implementation doesn't actually unwrap the result as described in supabase/supabase-js#92,
i.e. returning only the actual data. This is done deliberately for two reasons:

1. Making sure the caller can still extract `count`, `status` and
`statusText` while still benefitting from errors being thrown instead of
returned
2. Ease the transition from the non-throwing pattern where destructuring is
norm anyway, so less code has to be rewritten to take advantage of unwrap

Basic tests were added to test/basic.ts and the unwrap function is
documented to some degree at least, though this can probably be improved.

* Rename the unwrap API to throwOnError instead

The `unwrap` moniker was a bit of a misnomer, since it didn't actually
unwrap the result, and this should make things a bit more clear. This was
discussed in a bit more detail in supabase/postgrest-js#188.
---
 src/lib/types.ts                      | 20 +++++++++++++++++++-
 test/__snapshots__/index.test.ts.snap | 11 +++++++++++
 test/basic.ts                         | 27 +++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/src/lib/types.ts b/src/lib/types.ts
index d41bc074..a83707d3 100644
--- a/src/lib/types.ts
+++ b/src/lib/types.ts
@@ -54,11 +54,23 @@ export abstract class PostgrestBuilder<T> implements PromiseLike<PostgrestRespon
   protected headers!: { [key: string]: string }
   protected schema?: string
   protected body?: Partial<T> | Partial<T>[]
+  protected shouldThrowOnError?: boolean
 
   constructor(builder: PostgrestBuilder<T>) {
     Object.assign(this, builder)
   }
 
+  /**
+   * If there's an error with the query, throwOnError will reject the promise by
+   * throwing the error instead of returning it as part of a successful response.
+   *
+   * {@link https://github.com/supabase/supabase-js/issues/92}
+   */
+  throwOnError(): PostgrestBuilder<T> {
+    this.shouldThrowOnError = true
+    return this
+  }
+
   then<TResult1 = PostgrestResponse<T>, TResult2 = never>(
     onfulfilled?:
       | ((value: PostgrestResponse<T>) => TResult1 | PromiseLike<TResult1>)
@@ -92,7 +104,8 @@ export abstract class PostgrestBuilder<T> implements PromiseLike<PostgrestRespon
           const isReturnMinimal = this.headers['Prefer']?.split(',').includes('return=minimal')
           if (this.method !== 'HEAD' && !isReturnMinimal) {
             const text = await res.text()
-            if (text && text !== '' &&  this.headers['Accept'] !== 'text/csv') data = JSON.parse(text)
+            if (text && text !== '' && this.headers['Accept'] !== 'text/csv')
+              data = JSON.parse(text)
           }
 
           const countHeader = this.headers['Prefer']?.match(/count=(exact|planned|estimated)/)
@@ -102,6 +115,10 @@ export abstract class PostgrestBuilder<T> implements PromiseLike<PostgrestRespon
           }
         } else {
           error = await res.json()
+
+          if (error && this.shouldThrowOnError) {
+            throw error
+          }
         }
 
         const postgrestResponse: PostgrestResponse<T> = {
@@ -112,6 +129,7 @@ export abstract class PostgrestBuilder<T> implements PromiseLike<PostgrestRespon
           statusText: res.statusText,
           body: data,
         }
+
         return postgrestResponse
       })
       .then(onfulfilled, onrejected)
diff --git a/test/__snapshots__/index.test.ts.snap b/test/__snapshots__/index.test.ts.snap
index edb9dcf0..601d078d 100644
--- a/test/__snapshots__/index.test.ts.snap
+++ b/test/__snapshots__/index.test.ts.snap
@@ -626,6 +626,8 @@ Object {
 }
 `;
 
+exports[`connection errors should work the same with throwOnError 1`] = `[FetchError: request to http://this.url.does.not.exist/user?select=* failed, reason: getaddrinfo ENOTFOUND this.url.does.not.exist]`;
+
 exports[`don't mutate PostgrestClient.headers 1`] = `null`;
 
 exports[`embedded filters embedded eq 1`] = `
@@ -2262,3 +2264,12 @@ Object {
   "statusText": "OK",
 }
 `;
+
+exports[`throwOnError throws errors instead of returning them 1`] = `
+Object {
+  "code": "42P01",
+  "details": null,
+  "hint": null,
+  "message": "relation \\"public.missing_table\\" does not exist",
+}
+`;
diff --git a/test/basic.ts b/test/basic.ts
index 7b100e8a..a7084a47 100644
--- a/test/basic.ts
+++ b/test/basic.ts
@@ -95,6 +95,19 @@ test('missing table', async () => {
   expect(res).toMatchSnapshot()
 })
 
+test('throwOnError throws errors instead of returning them', async () => {
+  let isErrorCaught = false
+
+  try {
+    await postgrest.from('missing_table').select().throwOnError()
+  } catch (error) {
+    expect(error).toMatchSnapshot()
+    isErrorCaught = true
+  }
+
+  expect(isErrorCaught).toBe(true)
+})
+
 test('connection error', async () => {
   const postgrest = new PostgrestClient('http://this.url.does.not.exist')
   let isErrorCaught = false
@@ -107,6 +120,20 @@ test('connection error', async () => {
   expect(isErrorCaught).toBe(true)
 })
 
+test('connection errors should work the same with throwOnError', async () => {
+  const postgrest = new PostgrestClient('http://this.url.does.not.exist')
+  let isErrorCaught = false
+  await postgrest
+    .from('user')
+    .select()
+    .throwOnError()
+    .then(undefined, (error) => {
+      expect(error).toMatchSnapshot()
+      isErrorCaught = true
+    })
+  expect(isErrorCaught).toBe(true)
+})
+
 test('custom type', async () => {
   interface User {
     username: string