Re: Frontend error logging style

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I agree with this list of problems. I think that the end game here is
> getting to be able to use ereport() and friends in the frontend, which
> would require confronting both of these issues at a deep level. We
> don't necessarily have to do that now, though, but I think it's an
> argument against just nuking "fatal" from orbit. What I think we ought
> to be driving towards is having pg_log_fatal() forcibly exit, and
> pg_log_error() do the same unless the error is somehow caught.

Perhaps. The usage that I'm concerned about is exemplified by this
common pattern:

pg_log_error("%s", PQerrorMessage(conn));
PQfinish(conn);
exit(1);

ie there's some cleanup you want to do between emitting the error and
actually exiting. (I grant that maybe someday the PQfinish could be
done in an atexit handler or the like, but that's way more redesign
than I want to do right now.) In the case where you *don't* have
any cleanup to do, though, it'd be nice to just write one line not
two.

That leads me to think that we should redefine pg_log_fatal as

#define pg_log_fatal(...) (pg_log_error(__VA_ARGS__), exit(1))

We still want to get rid of the distinct PG_LOG_FATAL log level,
because whether you are using this form of pg_log_fatal or writing
exit() separately is a coding detail that should not manifest
as visibly distinct user output.

This proposal doesn't move us very far towards your endgame,
but I think it's a reasonable incremental step, and it makes
the difference between pg_log_error() and pg_log_fatal()
clear and useful.

> I have been wondering for some time about trying to make the frontend
> and backend facilities symmetric and using case to distinguish. That
> is, if you see:

> ERROR: this stinks
> DETAIL: It smells very bad.
> CONTEXT: garbage dump

> ...well then that's a backend message. And if you see:

> error: this stinks
> detail: It smells very bad.
> context: garbage dump

> ...well then that's a frontend message. I don't completely love that
> way of making a distinction, but I think we need something, and that's
> pretty nearly the present practice at least for the primary message.
> We don't really have a solid convention for detail/context/hint on the
> FE side; this is one idea.

Hmm, interesting. Taking up my point #2, I'd been thinking about
proposing that we convert

pg_log_error("query failed: %s", PQerrorMessage(conn));
pg_log_error("query was: %s", todo);

to

pg_log_error("query failed: %s", PQerrorMessage(conn));
pg_log_error_detail("Query was: %s", todo);

and similarly add a pg_log_info_detail() companion for pg_log_info(), etc.
With your point above, the prefix these'd print would be "detail:" not
"DETAIL:", but otherwise it seems to match up pretty well.

Also, if we are modeling this on backend conventions, we should say that
style rules for pg_log_error_detail match those for errdetail(), with
complete sentences and so forth. I wonder whether "detail:" followed
by a capitalized sentence would look weird ... but you did it above,
so maybe that's what people would expect.

Again, there's more that could be done later, but this seems to be
enough to address the cases that people have been inventing ad-hoc
solutions for.

regards, tom lane