Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't duplicate error messages for SyncError #6774

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Don't duplicate error messages for SyncError #6774

merged 1 commit into from
Jul 10, 2023

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Jul 7, 2023

SystemError is used in two places: as a base class for SyncError, and for filesystem errors. Filesystem errors don't include the actual error message and need it appended, but sync errors do and appending the message from the error code just results in it being duplicated.

SystemError is used in two places: as a base class for SyncError, and for
filesystem errors. Filesystem errors don't include the actual error message and
need it appended, but sync errors do and appending the message from the error
code just results in it being duplicated.
@tgoyne tgoyne requested a review from ironage July 7, 2023 20:44
@tgoyne tgoyne self-assigned this Jul 7, 2023
@cla-bot cla-bot bot added the cla: yes label Jul 7, 2023
@@ -1034,7 +1034,7 @@ void SimpleReporter::fail(const TestContext& context, const char* file, long lin
if (m_report_progress) {
m_error_messages.push_back(msg);
}
context.thread_context.report_logger.info(msg.c_str());
context.thread_context.report_logger.info("%1", msg.c_str());
Copy link
Member Author

Choose a reason for hiding this comment

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

The first argument to info() is a format string, so this was broken if the message contained %.

Copy link
Contributor

@ironage ironage left a comment

Choose a reason for hiding this comment

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

Thanks for finding this and fixing it! I should have been more careful to check the full message of the sync errors in #6756.

@tgoyne tgoyne merged commit e71aa03 into master Jul 10, 2023
@tgoyne tgoyne deleted the tg/sync-errors branch July 10, 2023 19:48
nicola-cab pushed a commit that referenced this pull request Jul 12, 2023
SystemError is used in two places: as a base class for SyncError, and for
filesystem errors. Filesystem errors don't include the actual error message and
need it appended, but sync errors do and appending the message from the error
code just results in it being duplicated.
nicola-cab pushed a commit that referenced this pull request Jul 12, 2023
SystemError is used in two places: as a base class for SyncError, and for
filesystem errors. Filesystem errors don't include the actual error message and
need it appended, but sync errors do and appending the message from the error
code just results in it being duplicated.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants