Обсуждение: Few untranslated error messages in OAuth

Hi,

During testing of libpq, I noticed several error messages not processed by
libpq_gettext, resulting in their exclusion from *.pot files. The attached patch
wraps these messages.

Best Regards,
Hou zj

On Thu, Nov 13, 2025 at 06:01:29AM +0000, Zhijie Hou (Fujitsu) wrote:
> During testing of libpq, I noticed several error messages not processed by
> libpq_gettext, resulting in their exclusion from *.pot files. The attached patch
> wraps these messages.

(Added Jacob and Daniel in CC.)

async_ctx says that errcxt "will be translated for you.", but these
are missing from the translation files.  So yes, it looks like you are
right.
--
Michael

On 2025-Nov-13, Zhijie Hou (Fujitsu) wrote:

> Hi,
> 
> During testing of libpq, I noticed several error messages not processed by
> libpq_gettext, resulting in their exclusion from *.pot files. The attached patch
> wraps these messages.

Yeah, this roughly makes sense.  I think the documentation for errctx in
async_ctx needs to be updated though, because currently it says "it'll
be translated for you" but this patch makes it the other way around (it
must come translated).

Alternatively, we could mark the strings with gettext_noop(), which only
adds the messages to the catalog without translating them; so the
responsibility of translating them would remain where it is today.  This
might be easier.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No tengo por qué estar de acuerdo con lo que pienso"
                             (Carlos Caszeli)

> On 13 Nov 2025, at 09:13, Álvaro Herrera <alvherre@kurilemu.de> wrote:
> On 2025-Nov-13, Zhijie Hou (Fujitsu) wrote:

>> During testing of libpq, I noticed several error messages not processed by
>> libpq_gettext, resulting in their exclusion from *.pot files. The attached patch
>> wraps these messages.

Thanks for the report!

> Yeah, this roughly makes sense.  I think the documentation for errctx in
> async_ctx needs to be updated though, because currently it says "it'll
> be translated for you" but this patch makes it the other way around (it
> must come translated).

+1

> Alternatively, we could mark the strings with gettext_noop(), which only
> adds the messages to the catalog without translating them; so the
> responsibility of translating them would remain where it is today.  This
> might be easier.

I was pondering that since auth error messages in backend libpq does this, but
since we don't use gettext_noop anywhere else in frontend libpq today it makes
sense to go with libpq_gettext and update the docs instead.

Unless you, who has more translation/nls insights than me, feel strongly about
gettext_noop I'll go ahead with the current patch.

--
Daniel Gustafsson

On 2025-Nov-13, Daniel Gustafsson wrote:

> I was pondering that since auth error messages in backend libpq does this, but
> since we don't use gettext_noop anywhere else in frontend libpq today it makes
> sense to go with libpq_gettext and update the docs instead.
>
> Unless you, who has more translation/nls insights than me, feel strongly about
> gettext_noop I'll go ahead with the current patch.

gettext_noop() does something completely different from libpq_gettext(),
and the more I think about this, the more I think doing libpq_gettext()
is the wrong thing.  I can give it a shot if you want.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Update: super-fast reaction on the Postgres bugs mailing list. The report
was acknowledged [...], and a fix is under discussion.
The wonders of open-source !"
             https://twitter.com/gunnarmorling/status/1596080409259003906

> On 13 Nov 2025, at 13:32, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Nov-13, Daniel Gustafsson wrote:
>
>> I was pondering that since auth error messages in backend libpq does this, but
>> since we don't use gettext_noop anywhere else in frontend libpq today it makes
>> sense to go with libpq_gettext and update the docs instead.
>>
>> Unless you, who has more translation/nls insights than me, feel strongly about
>> gettext_noop I'll go ahead with the current patch.
>
> gettext_noop() does something completely different from libpq_gettext(),
> and the more I think about this, the more I think doing libpq_gettext()
> is the wrong thing.

If so, should that be extended to oauth_json_set_error as well? (and possibly others?)

> I can give it a shot if you want.

If you have time, that would be great.

--
Daniel Gustafsson

On Thu, Nov 13, 2025 at 4:49 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> If so, should that be extended to oauth_json_set_error as well? (and possibly others?)

That should be covered already by nls.mk, unless I messed that up
too... I'll do a deeper dive.

Another way to fix this would be to wrap the assignment to errctx in a
macro that nls.mk can key off of, like oauth_json_set_error() does.
But I'm curious what Alvaro has in mind?

--Jacob

On Thu, Nov 13, 2025 at 8:23 AM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> On Thu, Nov 13, 2025 at 4:49 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> > If so, should that be extended to oauth_json_set_error as well? (and possibly others?)
>
> That should be covered already by nls.mk, unless I messed that up
> too... I'll do a deeper dive.

I misread your email, sorry. We trigger on oauth_parse_set_error(),
but not oauth_json_set_error(). I'll see how easy that is to change.

There's more to fix here, after talking with Alvaro offlist. I got
gettext to tell me [1] what else was missing and found that
- fe-auth-oauth.c is not in GETTEXT_FILES at all
- I didn't give jsonapi.c the same handling as in 3ddbac368 when I
ported it to libpq, so it's not translated either

I'm writing up a patch that combines all of that.

Thanks,
--Jacob

[1] https://www.gnu.org/software/gettext/manual/html_node/Prioritizing-messages.html

On Thu, Nov 13, 2025 at 3:08 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> I'm writing up a patch that combines all of that.

And here's what I have so far. I could use help double-checking the
.po result; I'm not entirely sure my usage of update-po is correct.

I took this opportunity to address a complaint from Alvaro and Peter E
a little while back, that some of the more internal-facing error
messages are very difficult to translate (and wouldn't help users even
if translation were easy). I can pull that (v2-0002) into its own
thread if necessary.

- v2-0001: combines Zhijie Hou's patch with the gettext_noop()
suggestion from Alvaro and fixes the nls.mk omission
- v2-0002: removes translation of multiplexer failures by adding an
_internal macro
- v2-0003: aligns oauth_json_set_error() with the prior commits
- v2-0004: tries to get jsonapi.c translated too

Unfortunately v2-0004 doesn't work. It puts the messages into the
translation files, but we use the _() macro throughout jsonapi.c,
which isn't helpful for libpq because libpq uses its own text domain.
This was an oversight in the work done for 0785d1b8b, I think, and it
may need its own patchset unless someone has a really quick fix.

Please let me know if I forgot to address something already said
above. I assume translation changes such as these are generally
backportable?

Thanks,
--Jacob

Hello,

On 2025-Nov-13, Jacob Champion wrote:

> On Thu, Nov 13, 2025 at 3:08 PM Jacob Champion
> <jacob.champion@enterprisedb.com> wrote:
> > I'm writing up a patch that combines all of that.
> 
> And here's what I have so far. I could use help double-checking the
> .po result; I'm not entirely sure my usage of update-po is correct.

I'm not sure what you think you might be doing wrong, but as far as I
can tell, the .po is being generated correctly.

> - v2-0001: combines Zhijie Hou's patch with the gettext_noop()
> suggestion from Alvaro and fixes the nls.mk omission
> - v2-0002: removes translation of multiplexer failures by adding an
> _internal macro
> - v2-0003: aligns oauth_json_set_error() with the prior commits

I gave these a quick look, and they look correct to me.  I didn't test
the resulting libpq though.

There are a few single quotes in some messages, which we tend not to
use.  I'd replace those with double quotes.  Of those, as an example,
this one caught my attention for strange wording,
  "internal error: field \"%s\" still active at end of object"
I think it means we haven't seen the closing quote at the end of the
field, right?  Maybe say something like "unterminated field \"%s\" ..."?

There's also the strings in CHECK_MSETOPT and siblings macros missing
quotes -- should be
   "failed to set \"%s\" on OAuth connection: %s"


> - v2-0004: tries to get jsonapi.c translated too
> 
> Unfortunately v2-0004 doesn't work. It puts the messages into the
> translation files, but we use the _() macro throughout jsonapi.c,
> which isn't helpful for libpq because libpq uses its own text domain.
> This was an oversight in the work done for 0785d1b8b, I think, and it
> may need its own patchset unless someone has a really quick fix.

You're right, that's no good.  We could try to define a new macro (maybe
jsonapi_gettext()) that does stock _() on backend but libpq_gettext() on
frontend ... but that wouldn't work nicely for frontend users other than
libpq.  Maybe something like

#ifndef jsonapi_gettext
#ifdef FRONTEND
#define jsonapi_gettext(msg) libpq_gettext(msg)
#else
#define jsonapi_gettext(msg) gettext(msg)
#endif
#endif

so any callers that want a third definition can just define it
themselves in the compile line?

Thanks for your time on this,

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Cómo ponemos nuestros dedos en la arcilla del otro. Eso es la amistad; jugar
al alfarero y ver qué formas se pueden sacar del otro" (C. Halloway en
La Feria de las Tinieblas, R. Bradbury)

On Thu, Nov 27, 2025 at 10:24 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> I gave these a quick look, and they look correct to me.  I didn't test
> the resulting libpq though.

Thanks for the review!

> Of those, as an example,
> this one caught my attention for strange wording,
>   "internal error: field \"%s\" still active at end of object"
> I think it means we haven't seen the closing quote at the end of the
> field, right?  Maybe say something like "unterminated field \"%s\" ..."?

Oh, those catch logic errors in the parsing engine. v3-0002 removes
those from the translation files as well.

> There's also the strings in CHECK_MSETOPT and siblings macros missing
> quotes -- should be
>    "failed to set \"%s\" on OAuth connection: %s"

Personally I prefer bare %s there, since it's an option name. Compare

    setsockopt(SO_REUSEADDR) failed
    failed to set CURLMOPT_SOCKETDATA on OAuth connection

> You're right, that's no good.  We could try to define a new macro (maybe
> jsonapi_gettext()) that does stock _() on backend but libpq_gettext() on
> frontend ... but that wouldn't work nicely for frontend users other than
> libpq.  Maybe something like
>
> #ifndef jsonapi_gettext
> #ifdef FRONTEND
> #define jsonapi_gettext(msg) libpq_gettext(msg)
> #else
> #define jsonapi_gettext(msg) gettext(msg)
> #endif
> #endif
>
> so any callers that want a third definition can just define it
> themselves in the compile line?

Yeah, the pattern should probably follow that of the
JSONAPI_USE_PQEXPBUFFER conditionals. I think I'll defer this until
after [1]; otherwise I might need to solve it twice. 0004 has been
dropped from the set.

On Thu, Nov 13, 2025 at 4:58 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> I assume translation changes such as these are generally
> backportable?

For now, I'll proceed as if a backport to 18 is appropriate for these.

Thanks again!
--Jacob

[1] https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com

Hello,

These patches look good to me.

On 2025-Dec-11, Jacob Champion wrote:

> Oh, those catch logic errors in the parsing engine. v3-0002 removes
> those from the translation files as well.

Sounds good.

> On Thu, Nov 27, 2025 at 10:24 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> > There's also the strings in CHECK_MSETOPT and siblings macros missing
> > quotes -- should be
> >    "failed to set \"%s\" on OAuth connection: %s"
> 
> Personally I prefer bare %s there, since it's an option name. Compare
> 
>     setsockopt(SO_REUSEADDR) failed
>     failed to set CURLMOPT_SOCKETDATA on OAuth connection

Hmm, okay.

> Yeah, the pattern should probably follow that of the
> JSONAPI_USE_PQEXPBUFFER conditionals. I think I'll defer this until
> after [1]; otherwise I might need to solve it twice. 0004 has been
> dropped from the set.
>
> [1] https://www.postgresql.org/message-id/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com

Sure, no objections to that plan.  I'd say these messages are probably
among the most important ones to translate in the OAuth support in
libpq, because as I understand some of them are more likely to become
part of a GUI that an end-user interacts with.  But I have no quarrels
with it all waiting until a later release.  (After all, early adopters
are going to force English on themselves anyway.)

No strong opinion on JSONAPI_USE_PQEXPBUFFER.  As far as I can tell, we
pretty much force you to link libpq if you want to have a PQExpBuffer,
which tells me that a frontend jsonapi.c user would already be forced to
link libpq.  So they will also have libpq_gettext().  So maybe a dual
answer is realistic -- no need to consider a third case of frontend
jsonapi.c without libpq.  If somebody in the future wants to use
jsonapi.c without libpq, they can do the translation API fix work then.

> On Thu, Nov 13, 2025 at 4:58 PM Jacob Champion
> <jacob.champion@enterprisedb.com> wrote:
> > I assume translation changes such as these are generally
> > backportable?
> 
> For now, I'll proceed as if a backport to 18 is appropriate for these.

Yeah, I'd prefer that.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)