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

batcher: major error handling overhaul #45

Merged
merged 2 commits into from
Jun 1, 2024
Merged

Conversation

abraithwaite
Copy link
Contributor

We previously were trying to handle errors either in Run() and previous to that: Send(). Unfortunately neither are great.

First when we tried send, the issue is that because batcher is async, the errors returned would have no semantic relation to the messages passed into the Send function call that just occurred. It was a way to pass errors back to the processor, but it leads to error handling which is error prone.

Next, we tried returning it from Run, but similarly the caller of Run has no context as to what it should do with the error.

Now, we're making error handling more explicit by requiring a handler get passed into the batcher. The signature allows for fairly flexible error handling and includes the batch that failed to flush.

Returning an error from this new function will be an indicator to the batcher to stop processing and return the error to Run(), mimicking the current functionality, but with a lot more clarity and explicitness.

No error returned from the error handler means that the events were processed in a manner appropriate to the error, whether that be dropped, archived for later, or retried.

Be wary about retrying at this stage. If you can rely on the source redelivering the message, better that than retrying here and putting backpressure on the queue. Also better to archive or send to a deadletter queue for retries.

We previously were trying to handle errors either in
Run() and previous to that: Send().  Unfortunately neither are great.

First when we tried send, the issue is that because batcher is async,
the errors returned would have no semantic relation to the messages
passed into the Send function call that just occurred.  It was _a_ way
to pass errors back to the processor, but it leads to error handling
which is error prone.

Next, we tried returning it from Run, but similarly the caller of `Run`
has no context as to what it should do with the error.

Now, we're making error handling more explicit by requiring a handler
get passed into the batcher.  The signature allows for fairly flexible
error handling and includes the batch that failed to flush.

Returning an error from this new function will be an indicator to the
batcher to stop processing and return the error to Run(), mimicking the
current functionality, but with a lot more clarity and explicitness.

No error returned from the error handler means that the events were
processed in a manner appropriate to the error, whether that be dropped,
archived for later, or retried.

Be wary about retrying at this stage.  If you can rely on the source
redelivering the message, better that than retrying here and putting
backpressure on the queue.  Also better to archive or send to a
deadletter queue for retries.
@abraithwaite abraithwaite merged commit 2a9c7aa into main Jun 1, 2024
1 check failed
@abraithwaite abraithwaite deleted the alan/batcher-errors branch June 1, 2024 01:18
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.

1 participant