From 62093457b3162fea0a5de8836c48a3e1d7998f1b Mon Sep 17 00:00:00 2001 From: Marcus Stade Date: Thu, 3 Jun 2021 22:47:58 +0200 Subject: [PATCH 1/2] 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. --- 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 153b6d8b..f386a2fc 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -53,11 +53,23 @@ export abstract class PostgrestBuilder implements PromiseLike | Partial[] + protected unwrapError?: boolean constructor(builder: PostgrestBuilder) { Object.assign(this, builder) } + /** + * If there's an error with the query, unwrap 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} + */ + unwrap(): PostgrestBuilder { + this.unwrapError = true + return this + } + then, TResult2 = never>( onfulfilled?: | ((value: PostgrestResponse) => TResult1 | PromiseLike) @@ -91,7 +103,8 @@ export abstract class PostgrestBuilder implements PromiseLike implements PromiseLike = { @@ -111,6 +128,7 @@ export abstract class PostgrestBuilder implements PromiseLike { expect(res).toMatchSnapshot() }) +test('unwrap throws errors instead of returning them', async () => { + let isErrorCaught = false + + try { + await postgrest.from('missing_table').select().unwrap() + } 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 error when unwrapping too', async () => { + const postgrest = new PostgrestClient('http://this.url.does.not.exist') + let isErrorCaught = false + await postgrest + .from('user') + .select() + .unwrap() + .then(undefined, error => { + expect(error).toMatchSnapshot() + isErrorCaught = true + }) + expect(isErrorCaught).toBe(true) +}) + test('custom type', async () => { interface User { username: string From 3a614846cc9579ab8f70acc9ea9da5efddde6bbd Mon Sep 17 00:00:00 2001 From: Marcus Stade Date: Fri, 11 Jun 2021 18:39:45 +0200 Subject: [PATCH 2/2] 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 | 10 +++++----- test/__snapshots__/index.test.ts.snap | 4 ++-- test/basic.ts | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/lib/types.ts b/src/lib/types.ts index f386a2fc..8082bc85 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -53,20 +53,20 @@ export abstract class PostgrestBuilder implements PromiseLike | Partial[] - protected unwrapError?: boolean + protected shouldThrowOnError?: boolean constructor(builder: PostgrestBuilder) { Object.assign(this, builder) } /** - * If there's an error with the query, unwrap will reject the promise by + * 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} */ - unwrap(): PostgrestBuilder { - this.unwrapError = true + throwOnError(): PostgrestBuilder { + this.shouldThrowOnError = true return this } @@ -115,7 +115,7 @@ export abstract class PostgrestBuilder implements PromiseLike { expect(res).toMatchSnapshot() }) -test('unwrap throws errors instead of returning them', async () => { +test('throwOnError throws errors instead of returning them', async () => { let isErrorCaught = false try { - await postgrest.from('missing_table').select().unwrap() + await postgrest.from('missing_table').select().throwOnError() } catch (error) { expect(error).toMatchSnapshot() isErrorCaught = true @@ -120,14 +120,14 @@ test('connection error', async () => { expect(isErrorCaught).toBe(true) }) -test('connection error when unwrapping too', async () => { +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() - .unwrap() - .then(undefined, error => { + .throwOnError() + .then(undefined, (error) => { expect(error).toMatchSnapshot() isErrorCaught = true })