Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: throwOnError function call should return response success type always #454

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
51 changes: 25 additions & 26 deletions src/PostgrestBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// @ts-ignore
import nodeFetch from '@supabase/node-fetch'

import type { Fetch, PostgrestSingleResponse } from './types'
import PostgrestError from './PostgrestError'
import type { Fetch, PostgrestResponseSuccess, PostgrestSingleResponse } from './types'

export default abstract class PostgrestBuilder<Result>
implements PromiseLike<PostgrestSingleResponse<Result>>
Expand All @@ -12,7 +11,6 @@ export default abstract class PostgrestBuilder<Result>
protected headers: Record<string, string>
protected schema?: string
protected body?: unknown
protected shouldThrowOnError = false
protected signal?: AbortSignal
protected fetch: Fetch
protected isMaybeSingle: boolean
Expand All @@ -23,7 +21,6 @@ export default abstract class PostgrestBuilder<Result>
this.headers = builder.headers
this.schema = builder.schema
this.body = builder.body
this.shouldThrowOnError = builder.shouldThrowOnError
this.signal = builder.signal
this.isMaybeSingle = builder.isMaybeSingle

Expand All @@ -39,12 +36,20 @@ export default abstract class PostgrestBuilder<Result>
/**
* 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.
* also return `PostgrestResponseSuccess` type for correct type inference
*
* {@link https://github.com/supabase/supabase-js/issues/92}
* {@link https://github.com/supabase/supabase-js/issues/801}
*/
throwOnError(): this {
this.shouldThrowOnError = true
return this
throwOnError<T extends Result = Result>(): PromiseLike<PostgrestResponseSuccess<T>> {
return this.then<PostgrestResponseSuccess<T>>(
(response) => {
if (response.error) throw response.error
return response as PostgrestResponseSuccess<T>
},
(error) => {
throw error
}
)
}

then<TResult1 = PostgrestSingleResponse<Result>, TResult2 = never>(
Expand Down Expand Up @@ -155,10 +160,6 @@ export default abstract class PostgrestBuilder<Result>
status = 200
statusText = 'OK'
}

if (error && this.shouldThrowOnError) {
throw new PostgrestError(error)
}
}

const postgrestResponse = {
Expand All @@ -171,20 +172,18 @@ export default abstract class PostgrestBuilder<Result>

return postgrestResponse
})
if (!this.shouldThrowOnError) {
res = res.catch((fetchError) => ({
error: {
message: `${fetchError?.name ?? 'FetchError'}: ${fetchError?.message}`,
details: `${fetchError?.stack ?? ''}`,
hint: '',
code: `${fetchError?.code ?? ''}`,
},
data: null,
count: null,
status: 0,
statusText: '',
}))
}
res = res.catch((fetchError) => ({
error: {
message: `${fetchError?.name ?? 'FetchError'}: ${fetchError?.message}`,
details: `${fetchError?.stack ?? ''}`,
hint: '',
code: `${fetchError?.code ?? ''}`,
},
data: null,
count: null,
status: 0,
statusText: '',
}))

return res.then(onfulfilled, onrejected)
}
Expand Down
34 changes: 19 additions & 15 deletions test/basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,13 @@ test('throwOnError throws errors instead of returning them', async () => {
expect(isErrorCaught).toBe(true)
})

test('throwOnError should return response success type', async () => {
const { data, error } = await postgrest.from('users').select().throwOnError()

expect(data).not.toBeNull()
expect(error).toBeNull()
})

test('throwOnError throws errors which include stack', async () => {
try {
await postgrest.from('does_not_exist').select().throwOnError()
Expand Down Expand Up @@ -692,25 +699,22 @@ test('throwOnError throws errors which include stack', async () => {
test('connection error w/o throwing', async () => {
const postgrest = new PostgrestClient<Database>('http://foo.invalid')
let isErrorCaught = false
await postgrest
.from('users')
.select()
.then(undefined, () => {
isErrorCaught = true
})
try {
await postgrest.from('users').select()
} catch (_) {
isErrorCaught = true
}
expect(isErrorCaught).toBe(false)
})

test('connection error w/ throwOnError', async () => {
const postgrest = new PostgrestClient<Database>('http://foo.invalid')
let isErrorCaught = false
await postgrest
.from('users')
.select()
.throwOnError()
.then(undefined, () => {
isErrorCaught = true
})
try {
await postgrest.from('users').select().throwOnError()
} catch (_) {
isErrorCaught = true
}
expect(isErrorCaught).toBe(true)
})

Expand All @@ -720,8 +724,8 @@ test('maybeSingle w/ throwOnError', async () => {
.from('messages')
.select()
.eq('message', 'i do not exist')
.throwOnError()
.maybeSingle()
.throwOnError()
.then(undefined, () => {
passes = false
})
Expand Down Expand Up @@ -1418,4 +1422,4 @@ test('update with no match - return=representation', async () => {
"statusText": "OK",
}
`)
})
})
4 changes: 2 additions & 2 deletions test/transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,10 @@ test('abort signal', async () => {
"count": null,
"data": null,
"error": Object {
"code": "",
"code": "20",
"details": Any<String>,
"hint": "",
"message": "AbortError: The user aborted a request.",
"message": "AbortError: This operation was aborted",
},
"status": 0,
"statusText": "",
Expand Down