On Tue, Jun 22, 2021 at 10:59:37PM +0000, Jacob Champion wrote:
> Hmm. I'm honestly hesitant to start splitting files apart, mostly
> because json_log_and_abort() is only called from two places, and both
> those places are triggered by programmer error as opposed to user
> error.
>
> Would it make more sense to switch to an fprintf-and-abort case, to
> match the approach taken by PGTHREAD_ERROR and the out-of-memory
> conditions in fe-print.c? Or is there already precedent for handling
> can't-happen code paths with in-band errors, through the PGconn?
Not really..
Looking more closely at that, I actually find a bit crazy the
requirement for any logging within jsonapi.c just to cope with the
fact that json_errdetail() and report_parse_error() just want to track
down if the caller is giving some garbage or not, which should never
be the case, really. So I would be tempted to eliminate this
dependency to begin with.
The second thing is how we should try to handle the way the error
message gets allocated in json_errdetail(). libpq cannot rely on
psprintf(), so I can think about two options here:
- Let the caller of json_errdetail() allocate the memory area of the
error message by itself, pass it down to the function.
- Do the allocation within json_errdetail(), and let callers cope the
case where json_errdetail() itself fails on OOM for any frontend code
using it.
Looking at HEAD, the OAUTH patch and the token handling, the second
option looks more interesting. I have to admit that handling the
token part makes the patch a bit special, but that avoids duplicating
all those error messages for libpq. Please see the idea as attached.
--
Michael