Update WriteErrors error message #964
Conversation
achille-roussel
left a comment
achille-roussel
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
|
Hi @achille-roussel, are we going to merge this pr? |
error.go
Outdated
| func (err WriteErrors) Error() string { | ||
| return fmt.Sprintf("kafka write errors (%d/%d)", err.Count(), len(err)) | ||
| var sb strings.Builder | ||
| for i, writeError := range err { |
There was a problem hiding this comment.
Can write error ever be nil here?
There was a problem hiding this comment.
i don't think so, if err is empty then it will never execute it what do you think? @rhansen2
There was a problem hiding this comment.
It is not possible for one element of the array to be nil?
There was a problem hiding this comment.
Based on what's documented here I believe we have to handle the case when an item in the array is nil.
There was a problem hiding this comment.
@rhansen2 thanks you are right, I added validation of that.
error.go
Outdated
| if writeError == nil { | ||
| continue | ||
| } | ||
| fmt.Fprintf(&sb, "Message %d: %s \n", i, writeError.Error()) |
There was a problem hiding this comment.
Sorry, one list nit, it looks like the individual errors are at batch level not individual message level, Message here might be a bit misleading.
There was a problem hiding this comment.
@rhansen2 I removed the string builder part, instead of that I used a slice to be cleaner what do you think about it?
* Update WriteErrors error message * Update error.go * Handle error nil case * Update error message for write errors * preallocate the errors slice Co-authored-by: Achille <[email protected]>
* Update WriteErrors error message * Update error.go * Handle error nil case * Update error message for write errors * preallocate the errors slice Co-authored-by: Achille <[email protected]>
3 participants
We can't see the actual errors of our batch write request. It creates new error and this error does not contain any information about the error return fmt.Sprintf("kafka write errors (%d/%d)", err.Count(), len(err)). We need to see the detail of the error.
Issue thread: #957