> On 27 Mar 2026, at 18:01, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>
> On Mon, Mar 23, 2026 at 2:21 PM Zsolt Parragi <zsolt.parragi@percona.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
