Skip to content

Fix gbak output some errors and warnings to stderr instead of stdout#8793

Open
XaBbl4 wants to merge 2 commits intoFirebirdSQL:masterfrom
XaBbl4:fix_gbak_stderr
Open

Fix gbak output some errors and warnings to stderr instead of stdout#8793
XaBbl4 wants to merge 2 commits intoFirebirdSQL:masterfrom
XaBbl4:fix_gbak_stderr

Conversation

@XaBbl4
Copy link
Contributor

@XaBbl4 XaBbl4 commented Oct 29, 2025

Currently in standalone application mode when redirect to the standard stream, for example:
gbak ... > /path/to/stdout.log
some error and warning messages may be missed, which may cause inconvenience.

For example, during when restore we may see an error message:

gbak: ERROR:Database is not online due to failure to activate one or more indices.
gbak: ERROR:    Run gfix -online to bring database online without active indices.

but in order to find out which index remains inactive, we need to look at the entire log, and in it we can already find:

gbak:cannot commit index TEST_INDEX
gbak: ERROR:attempt to store duplicate value (visible to active transactions) in unique index "TEST_INDEX"
gbak: ERROR:    Problematic key value is ("ID" = 1)

Although in the global community it is accepted to output all errors and warnings to stderr.

This patch fixes this bug. I tried to cover all the cases found, but may have missed something.
I believe BURP_print_status should always output to stderr, regardless of whether the err argument is set to true or not. The err argument is now only responsible for setting status in service mode. Also BURP_print_warning should always output to stderr.

@dyemanov dyemanov requested a review from AlexPeshkoff March 5, 2026 06:22
@AlexPeshkoff
Copy link
Member

Earlier the following approach was used - errors that may be recovered by gbak were going to stdout, fatal - to stderr. I can agree that putting recoverable errors into stderr makes sense. Bit why not all? Something like:

         BURP_print(false, 171, relation->rel_name.toQuotedString().c_str());
         // msg 171: error committing metadata for relation %s

in restore.epp is left in stdout but

         BURP_print(true, 343, SafeArg() << int(attribute) << "put_asciz()" << (MAX_FILE_NAME_SIZE - 1));
         // msg 343: text for attribute @1 is too large in @2, truncating to @3 bytes

in backup.epp will go into stderr.

I want to know what criteria was used to keep some messages in stdout.

@XaBbl4
Copy link
Contributor Author

XaBbl4 commented Mar 5, 2026

Initially, during the restore, I tried to capture only those errors that didn't bring the database online when the utility terminated.

As I mentioned in the first message, I might have missed some places, as I couldn't reproduce every case; this is likely one of them.

In the backup, I moved all BURP_print calls to the error stream, not seeing the logic for splitting the output.
As for error 343, the code had a similar output in the put_text function, and put_asciz also sent it to the error stream.

Copy link
Member

@AlexPeshkoff AlexPeshkoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Andrey, may be I was not enough explicit. I suggest to:

  1. Complete cleanup process in restore.epp. There is no need to reproduce all bugs - just find all BURP_print calls with false parameter. Regarding previous comments about about errors 171 & 343 - it's completely OK to sent output to stderr for both of them. Like all other invocations of BURP_error.

  2. After cleanup finished - may be argument err should be removed from BURP_print() at all? For a few remaining calls in burp.cpp like
    BURP_print(false, 91, FB_VERSION);
    may be invite new function name?

All messages from these functions are now printed to standard error stream.
BURP_print with output version has been replaced by BURP_message (same result).
Calls BURP_error_redirect with NULL status argument replaces BURP_error with abort argument, because calling BURP_print_status only adds call overhead.
@XaBbl4
Copy link
Contributor Author

XaBbl4 commented Mar 16, 2026

  1. Remove err argument from BURP_print and BURP_print_status. All messages from these functions are now printed to standard error stream. In BURP_print_status add serviceFlag argument for set service code to service and don't print to standart streams.

  2. BURP_print with output version has been replaced to BURP_message.

  3. In additional calls BURP_error_redirect with NULL status argument replaced to BURP_error with abort argument, because calling BURP_print_status only adds small overhead.

  4. I have one question about the output of msg 81. There are two outputs in the get_array function, but one had BURP_print_status(true, ...) and the other had BURP_print_status(false, ...). I left it as is for now. According to history, you did this in commit 7403516, and it looks like there was a typo, or maybe I just missed the sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants