search

agroal / pgagroal

High-performance connection pool for PostgreSQL
https://agroal.github.io/pgagroal/
BSD 3-Clause "New" or "Revised" License
667 stars 59 forks source link

`pgagroal-cli` reports wrong (JSON) output when there is a management error #412

Closed fluca1978 closed 3 months ago

fluca1978 commented 4 months ago

While reviewing #411 I found that, if pgagroal-cli encounters an error in the management phase, it does not report the JSON output even if requested, and reports instead a misleading success message.

% pgagroal-cli conf set max_connections 200 --format json
 TRACE cli.c:691 <conf set> [max_connections] = [200]
WARN  management.c:1837 pgagroal_management_read_config_get : error retrieving configuration for <max_connections> : Success

Tested on d9f9253504605194b6bac1dd18491f132059b145.

Log on the daemon side:

DEBUG main.c:1558 pgagroal: Management config-set for key <max_connections> setting value to <200>
DEBUG management.c:1948 pgagroal_management_write_config_set: trying to set <max_connections> to <200>
DEBUG configuration.c:4454 Trying to change main configuration setting <max_connections> to <200>
INFO  configuration.c:2442 Restart required for max_connections - Existing 15 New 200
WARN  configuration.c:2369 1 settings cannot be applied
DEBUG management.c:1953 pgagroal_management_write_config_set: unable to apply changes to <max_connections> -> <200>

Need some investigation in order to make the output to appear as JSON (when required) reporting the correct error message. However, it seems this is not a problem of JSON output strictly, since the application seems to misbehave even with output text:

% pgagroal-cli conf set max_connections 200
TRACE cli.c:466 Command: <conf set> [max_connections] = [200]
WARN  management.c:1837 pgagroal_management_read_config_get : error retrieving configuration for <max_connections> : Success

% echo $?
2
fluca1978 commented 4 months ago

The problem is in interpreting a faulty JSON too early in the management protocol, for example https://github.com/agroal/pgagroal/blob/master/src/libpgagroal/management.c#L1805. If the JSON object if faulty, we don't need to raise an error at the management level, rather to propagate the faulty JSON to whoever requested it (setting the exist status accordingly).