| 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
| 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 |