Re: unclear OAuth error message

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: unclear OAuth error message
Date: 2026-03-27 22:24:40
Message-ID: E187E05E-33C2-46E1-A324-160374CE1914@yesql.se
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 27 Mar 2026, at 18:01, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>
> On Mon, Mar 23, 2026 at 2:21 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>> Isn't including the detail for both the warning and the fatal error
>> still overly verbose?
>
> I'm not too worried about verbosity for an internal error situation;
> users shouldn't see it. If they do, I don't mind being very loud about
> whose fault it is.
>
> (I'm also influenced by some recent support work on clusters that have
> huge log volumes. If someone is focused on the internal error, they
> should be able to see at a glance what caused that error, and if
> someone is focused on the authentication failure, they should be able
> to see at a glance what caused that. The more logs you have to
> correlate in a "help! no one can log in" panic situation, the less
> likely you are to succeed.)

Agreed.

>> Shouldn't the oauth code include a sanity check to ensure validators
>> return no error_detail on success instead of silently ignoring it?
>
> IMO, no. I don't want error_detail to add semantics to the API, just
> descriptive power. Plus, I think a design that sets a possible error
> message before entering a complex operation, knowing that it will be
> ignored on success, is perfectly valid. libpq-oauth, and to a lesser
> extent libpq, make use of that pattern.

Callsites can also clear the error message on success and not even rely on it
being ignored.

+ * This string may be either of static duration or palloc'd.
+ */
+ char *error_detail;

I'm not a big fan of "either static or allocated" and prefer if we just require
one or the other. We have this pattern in other places so it's not a blocker
for going it, but.

--
Daniel Gustafsson

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2026-03-27 22:37:33 Shared hash table allocations
Previous Message Nathan Bossart 2026-03-27 22:10:16 Re: Clean up NamedLWLockTranche stuff