fix(puller): prevent massive log lines from joined errors #5331
Conversation
gacevicljubisa
requested review from
acud,
akrem-chabchoub,
janos and
martinconic
janos
left a comment
janos
left a comment
There was a problem hiding this comment.
The approach is maybe too high level. It looks to me that errors.Join is used in a way that is not ideal, maybe even misused. This is solving the problem with long log lines, but does not solve the root cause of using errors as lists of errors.
I would say that this is ok solution but ideally it would be better in the future to address the usage of errors.Join.
| @@ -0,0 +1,72 @@ | |||
| // Copyright 2025 The Swarm Authors. All rights reserved. | |||
I agree. |
| errCount := countErrors(err) | ||
| if errCount > 1 { | ||
| p.logger.Debug("syncWorker interval failed", "error_count", errCount, "example_error", errors.Unwrap(err), "peer_address", address, "bin", bin, "cursor", cursor, "start", start, "topmost", top) | ||
| } else { |
There was a problem hiding this comment.
errors.Unwrap() looks for Unwrap() error, but errors.Join() implements Unwrap() []error.
Would this return nil here?
Checklist
Description
Previously, when many errors were joined together during sync
operations, all errors were logged in a single line, creating
log entries up to 30KB (e.g., 245 identical "batch not found"
errors).
This commit adds countErrors() to count total errors and logs
only the count plus one example error, keeping entries concise
while maintaining diagnostic value.
Changes:
Example output:
Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):