Return multiple errors from Writer.WriteMessages and MessageTooLargeError handling improvements #401
Conversation
…ct error channel to be retried.
Add id field so that a given message can be tracked back to its index when it was originally sent from Writer.WriteMessages. id allows WriteMessages to keep track of which messages have been successfully sent therefore making it easier to do more sophisticated error handling.
Change res channel type in writerMessage to new writerResponse struct type so ids can be returned back to writer.WriteMessages. Refactor affected receivers.
Implement WriterErrors type which implements the error interface to allow for the returning of multiple WriterErrors that may occur during a write. Provide basic Error() method which takes inspiration from https://github.com/hashicorp/go-multierror/blob/72917a1559e17f38638ade54020ab372ba848d67/format.go#L14 so the slice of WriterError is formatted in a nice fashion.
error is a bit too general for dependent code. All err values are WriterError anyway so make it the more specific type.
… before returning MessageTooLarge errors Utilize WriterErrors and WriterError types to return all errors that occur when sending a batch of messages using Writer.WriteMessages while still preserving error return type. Try to send all eligible messages in msgs before returning any MessageTooLarge errors.
Evanjt1
changed the title
| type WriterError struct { | ||
| Msg Message | ||
| Err error | ||
| } |
There was a problem hiding this comment.
Could we have the WriterError and WriterErrors type implement errors.Wrapper?
There was a problem hiding this comment.
Will do, Unwrap() is straightforward for WriterError. Do you have thoughts on behavior for WriterErrors?
There was a problem hiding this comment.
@achille-roussel For WriterErrors.Unwrap() how do you feel about using the same strategy as https://github.com/prometheus/client_golang/blob/82ce871c27ca8919bcdb97d69bcf461499171b27/prometheus/registry.go#L240?
Unwrap needs to be implemented in order to use the Unwrap function added in go 1.13.
# Conflicts: # writer.go
|
@achille-roussel This PR is pretty out-of-date at this point and your work on #438 covers the issues this PR was about so I think this should probably be closed. |
|
Thanks for follow up @Evanjt1, I'll go ahead and close. Feel free to reopen an issue if the problem isn't properly addressed. |
Addresses issues #363 and #364
This PR revolves around 2 main improvements for the Writer type:
Writer.WriteMessagesI introduced two new public types,
WriterErrorandWriterErrorsto implement the first improvement. Both implement the error interface. Under the hoodWriter.WriteMessagesreturnsWriterErrors. The caller can cast the error returned asWriterErrorsto use the new functionality if they so choose.The second improvement is addressed by skipping messages that are too large as the
Writercomes across them while sending to themsgschannel. They are instead stored as errors that will be returned after all the other messages have been tried.New tests have been added to
writer_test.goto ensure these features behave as expected. Additionally, some of the preexisting tests have been updated so they also check theWriterErrorsreturned.There would have been a bunch of merge conflicts and wasted work if I tried to made both improvements individually so I decided to knock out two birds with one stone.