fix(postgrest): return a structured error for non-JSON body on successful responses

[youcefzemmar]

Description

On the success path (res.ok === true), postgrest-js calls JSON.parse(body) with no error handling. A 2xx status does not guarantee a JSON body:

• A proxy / gateway / CDN in front of PostgREST (Cloudflare, nginx, a corporate proxy, a service worker, etc.) can return a non-JSON body — e.g. an HTML error page or a plaintext upstream connect error — with a success status.
• A connection dropped mid-stream yields a truncated body (Unexpected end of JSON input) even from a perfectly well-behaved server.

When that happens today the raw SyntaxError escapes processResponse, and the result depends on how the query was awaited:

1. With .throwOnError() → the promise rejects with a SyntaxError, not a PostgrestError. This breaks the documented contract — catch blocks that read error.code / error.hint / error.details receive an unexpected shape.
2. Without it → the error is caught by the network-error handler and returned as { status: 0, error: { message: "SyntaxError: Unexpected token ..." } }. status: 0 signals a client-side network failure, which is misleading: the request actually reached the server and returned a real HTTP status.

The non-2xx branch already tolerates non-JSON bodies (it falls back to error = { message: body }). This PR makes the success path symmetric with it.

Change

Wrap the success-path JSON.parse in a try/catch. On parse failure:

• return a structured error (message, details with a truncated body snippet, an actionable hint, empty code — matching the existing client-side error shape),
• preserve the real HTTP status / statusText instead of reporting status: 0,
• throw a PostgrestError when .throwOnError() is set.

Valid-JSON responses are completely unaffected — this is purely defensive.

Type of Change

• Bug fix (fix)

Testing

• Unit tests added (packages/core/postgrest-js/test/fetch-errors.test.ts, mocked fetch — no Docker): HTML body on 200, truncated JSON, real-status preservation, oversized-body truncation in details, and the throwOnError → PostgrestError contract.
• nx type-check / nx type-check:test pass.
• nx format:check passes; mock-based suites (fetch-errors, retry, timeout-and-url-length) pass.

Checklist

• Code formatted (nx format)
• Used conventional commits (fix(postgrest): ...)

---

Related context: supabase/postgrest-js#646 and supabase/postgrest-js#282 (the "Unexpected token < in JSON" class of reports). This PR intentionally scopes to client-side robustness/contract-correctness rather than expecting upstream proxies to always emit JSON.

🤖 Generated with Claude Code

[pkg-pr-new]

Open in StackBlitz

@supabase/auth-js

npm i https://pkg.pr.new/@supabase/auth-js@2398

@supabase/functions-js

npm i https://pkg.pr.new/@supabase/functions-js@2398

@supabase/postgrest-js

npm i https://pkg.pr.new/@supabase/postgrest-js@2398

@supabase/realtime-js

npm i https://pkg.pr.new/@supabase/realtime-js@2398

@supabase/storage-js

npm i https://pkg.pr.new/@supabase/storage-js@2398

@supabase/supabase-js

npm i https://pkg.pr.new/@supabase/supabase-js@2398

commit: 597a29d

[mandarini]

Thanks for the careful analysis, the two failure modes you identified
(SyntaxError escaping .throwOnError() and the misleading status: 0 when
the request actually reached the server) are real, and the tests are
well-targeted. A few things I'd like to see before this merges, all in the
direction of less code:

1. Mirror the existing non-2xx fallback exactly. The non-2xx branch
   already handles non-JSON bodies as error = { message: body } — three
   lines, no truncation, no hint, no custom message format. The success path
   should do the same so the two non-JSON branches produce the same error
   shape. Something like:

   try {
     data = JSON.parse(body)
   } catch {
     error = { message: body }
     data = null
     if (this.shouldThrowOnError) {
       throw new PostgrestError({ message: body, details: '', hint: '', code: '' })
     }
   }

   That fixes both bugs (the SyntaxError leak, and status: 0 — which is
   fixed implicitly because we no longer fall into the network-error handler)
   without introducing a third error shape alongside the existing PostgREST
   structured errors and the non-2xx fallback.

2. Drop the 500-byte snippet truncation, the hint paragraph, and the
   "Failed to parse … : <parseError.message>" message format. These are
   speculative UX polish, not part of the bug fix:

   • The hint guesses at the user's infrastructure ("proxy, gateway, or
     CDN…") — it could be wrong and is worse than no hint when it is.
   • The 500-byte cap is arbitrary and needs its own test (the one you
     wrote) just to defend that arbitrary choice.
   • The custom message format becomes a new string consumers may match
     against.

   If we later want a richer error shape, that's a separate refactor that
   should bring both the success and non-2xx fallbacks along together.

3. Drop the long inline comment. The rationale already lives in the PR
   description and the commit message — that's where readers go for "why
   this exists." A one-line comment is fine if it adds something the code
   doesn't already say.

With those changes the tests will simplify too, the truncation test goes
away, and the others stay roughly the same since the structured-error
assertions become error.message === <body> checks.

[youcefzemmar]

Thanks for the careful analysis, the two failure modes you identified (SyntaxError escaping .throwOnError() and the misleading status: 0 when the request actually reached the server) are real, and the tests are well-targeted. A few things I'd like to see before this merges, all in the direction of less code:

1. **Mirror the existing non-2xx fallback exactly.** The non-2xx branch
   already handles non-JSON bodies as `error = { message: body }` — three
   lines, no truncation, no hint, no custom message format. The success path
   should do the same so the two non-JSON branches produce the same error
   shape. Something like:
   ```ts
   try {
     data = JSON.parse(body)
   } catch {
     error = { message: body }
     data = null
     if (this.shouldThrowOnError) {
       throw new PostgrestError({ message: body, details: '', hint: '', code: '' })
     }
   }
   ```
   
   
       
         
       
   
         
       
   
       
     
   That fixes both bugs (the `SyntaxError` leak, and `status: 0` — which is
   fixed implicitly because we no longer fall into the network-error handler)
   without introducing a third error shape alongside the existing PostgREST
   structured errors and the non-2xx fallback.

2. **Drop the 500-byte snippet truncation, the `hint` paragraph, and the
   `"Failed to parse … : <parseError.message>"` message format.** These are
   speculative UX polish, not part of the bug fix:
   
   * The hint guesses at the user's infrastructure ("proxy, gateway, or
     CDN…") — it could be wrong and is worse than no hint when it is.
   * The 500-byte cap is arbitrary and needs its own test (the one you
     wrote) just to defend that arbitrary choice.
   * The custom message format becomes a new string consumers may match
     against.
   
   If we later want a richer error shape, that's a separate refactor that
   should bring both the success and non-2xx fallbacks along together.

3. **Drop the long inline comment.** The rationale already lives in the PR
   description and the commit message — that's where readers go for "why
   this exists." A one-line comment is fine if it adds something the code
   doesn't already say.

With those changes the tests will simplify too, the truncation test goes away, and the others stay roughly the same since the structured-error assertions become error.message === <body> checks.

as you asked

[mandarini]

Thank you!