feat: add Proxy-Status header for better error response

[taimoorzaeem]

With this feature, PostgREST responds with error code in the Proxy-Status header.

Closes #2967.

[steve-chavez]

Loadtest looking good, no perf loss https://github.com/PostgREST/postgrest/actions/runs/13937206620?pr=3955

[taimoorzaeem]

@steve-chavez @laurenceisla Currently, we assign error codes to errors at the time of generating the error json:

postgrest/src/PostgREST/Error.hs

Lines 227 to 231 in 3c0baec

	toJSON (InvalidPreferences prefs) = toJsonPgrstError
	ApiRequestErrorCode22
	"Invalid preferences given with handling=strict"
	(Just $ JSON.String $ T.decodeUtf8 ("Invalid preferences: " <> BS.intercalate ", " prefs))
	Nothing

The issue with that design is that we can't reuse the same assignment when generating Proxy-Status header without some duplication.

So, I think we need a refactor here where we assign codes somewhere else. I am thinking of refactoring in this way:

class PgrstError a where
  status   :: a -> HTTP.Status
  headers  :: a -> [Header]
  
  -- this will also ensure all error have these attributes at a typeclass level
  code    :: a -> ErrorCode
  message :: a -> Text
  details :: a -> Maybe JSON.Value
  hint    :: a -> Maybe JSON.Value

  errorPayload :: a -> LByteString
  errorPayload = JSON.encode

  errorResponseFor :: a -> Response
  errorResponseFor err =
    let baseHeader = MediaType.toContentType MTApplicationJSON in
    responseLBS (status err) (baseHeader : headers err) $ errorPayload err
    
-- then we can reuse these when generating the error json
instance JSON.ToJSON PgrstError where
  toJSON err = toJsonPgrstError (code err) (message err) (details err) (hint err)

[steve-chavez]

So, I think we need a refactor here where we assign codes somewhere else. I am thinking of refactoring in this way:

@taimoorzaeem LGTM, please go ahead.

[taimoorzaeem]

@steve-chavez Is returning Proxy-Status header on every response critically important? If so we would need to test this extensively. Currently I have written some tests but that's not ideal I guess. Any ideas on how I can test this in a better way?

[steve-chavez]

@steve-chavez Is returning Proxy-Status header on every response critically important? If so we would need to test this extensively.

It should be only on error responses. Given your refactors on #3987, I think it's not necessary to have so much extra testing since we already prove in code that all error responses will contain a code?

Given that, only some tests would suffice. Say just one proxy-status error for each different group? (not adamant on this if there are some error groups that are hard to repro, like the X00 ones).

[steve-chavez]

@taimoorzaeem Great work! 🚀

We should document this. Maybe on https://docs.postgrest.org/en/v12/references/observability.html#traces, after trace header.

[steve-chavez]

This is awesome. I just played it with it and it even works with custom errors:

# postgrest-with-postgresql-16  -f test/spec/fixtures/load.sql postgrest-run

$ curl localhost:3000/rpc/raise_pt402 -i
..
Proxy-Status: PostgREST; error=PT402

$ curl localhost:3000/rpc/raise_sqlstate_test1 -i
..
Proxy-Status: PostgREST; error=123

@taimoorzaeem How about adding tests for these two cases, to ensure they don't break? (fixtures already included, so it should be easy)

---

Also, I've noticed that the docs for Proxy-Status would better fit in the errors page too. It should mention that it will contain error codes from PostgREST, PostgreSQL and Custom Errors (no need for snippets on each one, just keeping the one about statement_timeout is good).

We can still keep a reference (link) to it in the Observability page, same place; for more visilibity.