Skip to content

Commit

Permalink
feat: Implement unwrap() to throw query errors instead of returning t…
Browse files Browse the repository at this point in the history
…hem (#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 #188.
  • Loading branch information
mstade committed Jun 11, 2021
1 parent 5ac1216 commit 9df1e84
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
20 changes: 19 additions & 1 deletion src/lib/types.ts
Expand Up @@ -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>)
Expand Down Expand Up @@ -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)/)
Expand All @@ -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> = {
Expand All @@ -112,6 +129,7 @@ export abstract class PostgrestBuilder<T> implements PromiseLike<PostgrestRespon
statusText: res.statusText,
body: data,
}

return postgrestResponse
})
.then(onfulfilled, onrejected)
Expand Down
11 changes: 11 additions & 0 deletions test/__snapshots__/index.test.ts.snap
Expand Up @@ -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`] = `
Expand Down Expand Up @@ -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",
}
`;
27 changes: 27 additions & 0 deletions test/basic.ts
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 9df1e84

Please sign in to comment.