PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.25k stars 1.02k forks source link

Customizing error messages on failed check constraints, Post-request hook #2706

Open steve-chavez opened 1 year ago

steve-chavez commented 1 year ago

Problem

Having:

CREATE DOMAIN public.filled_text AS text
    CONSTRAINT not_empty CHECK ((NOT (VALUE ~ '^\s*$'::text)));

COMMENT ON DOMAIN public.filled_text IS 'Text value that cannot be empty or whitespace, but doesn''t assume not null.';

A failed POST request will return:

{"code":"23514","details":null,"hint":null,"message":"value for domain filled_text violates check constraint \"not_empty\""}

Which is not user friendly. We've been telling users to format the error message on the client side, but this is not possible with direct consumers of the API(unless a proxy is involved, which complicates the setup).

Proposal

Just to kickstart discussion. We could have a pre-request function that does:

create or replace function custom_errors() returns void as $$
declare
  error_message text := current_setting('response.error', true)::json->>'message';
  error_code text := current_setting('response.error', true)::json->>'code';
begin
  if error_code = 23514 and error_message like '%not_empty%'
    then set_config(`response.error`, json_build_object('message', 'cannot be empty'); -- one problem here is that there's no way to know the column
  end if;
end; $$ language plpgsql;

Related: https://github.com/PostgREST/postgrest/issues/1438

mkleczek commented 1 year ago

Maybe https://datatracker.ietf.org/doc/rfc7807/ is relevant?

steve-chavez commented 1 year ago

Hm, how so? RFC 7807 is for the error message format right? We discussed adopting that on https://github.com/PostgREST/postgrest/pull/1926#issuecomment-916591592

wolfgangwalther commented 1 year ago

Just to kickstart discussion. We could have a pre-request function that does:

I don't understand how you could know at the time the pre-request function runs what kind of error to return? Doesn't this error just happen afterwards?

What about:

steve-chavez commented 9 months ago

On https://github.com/PostgREST/postgrest/issues/3084#issue-2022449376, @dvv mentioned:

I wonder if we could have a post-request db hook in addition to pre-request one? The rationale is to sanitize errors or to reconcile just in-place. It could be given all available info from get stacked diagnostics and raise more meaningful messages or just rewrite the json data.

Hm, I don't think we can use get stacked diagnostics since that would imply plpgsql?

dvv commented 9 months ago

@steve-chavez Right, catching the exception directly implies plpgsql wrapping by begin/exception/end which is not suitable.

But we could honor user-defined optional

create or replace function app_priv.on_response(result json = null, error json = null, statement text = null, out response json) as $$
-- process the successful result: eg. mangle field names, put to the cache for later quick fetch in app.authenticate() etc.
-- or process the error: look into hint/detail and deduce a friendlier error message, log the error at own will etc.
-- or just craft entirely new response: conditionally fail while debugging, whatever.
$$ stable ...;

and pass it the result, the entire SQL.ServerError if any, the statement if applicable.