On Mon, Jul 3, 2023 at 6:32 PM Euler Taveira <eu...@eulerto.com> wrote: > > On Mon, Jul 3, 2023, at 7:30 AM, Ashutosh Bapat wrote: > > logicalrep_message_type() is used to convert logical message type code > into name while prepared error context or details. Thus when this > function is called probably an ERROR is already raised. If > logicalrep_message_type() gets an unknown message type, it will throw > an error, which will suppress the error for which we are building > context or details. That's not useful. I think instead > logicalrep_message_type() should return "unknown" when it encounters > an unknown message type and let the original error message be thrown > as is. > > > Hmm. Good catch. The current behavior is: > > ERROR: invalid logical replication message type "X" > LOG: background worker "logical replication worker" (PID 71800) exited with > exit code 1 > > ... that hides the details. After providing a default message type: > > ERROR: invalid logical replication message type "X" > CONTEXT: processing remote data for replication origin "pg_16638" during > message type "???" in transaction 796, finished at 0/16266F8 >
I think after returning "???" from logicalrep_message_type(), the above is possible when we get the error: "invalid logical replication message type "X"" from apply_dispatch(), right? If so, then what about the case when we forgot to handle some message in logicalrep_message_type() but handled it in apply_dispatch()? Isn't it better to return the 'action' from the function logicalrep_message_type() for unknown type? That way the information could be a bit better and we can easily catch the code bug as well. -- With Regards, Amit Kapila.
