> On 27 Mar 2026, at 18:01, Jacob Champion <[email protected]> > wrote: > > On Mon, Mar 23, 2026 at 2:21 PM Zsolt Parragi <[email protected]> > 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
