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

Handle put events retry by default #22

Open
CorentinDoue opened this issue May 16, 2023 · 1 comment
Open

Handle put events retry by default #22

CorentinDoue opened this issue May 16, 2023 · 1 comment

Comments

@CorentinDoue
Copy link

Bus.put(events) returns { FailedEntryCount, Entries } which must be handled manually to avoid losing events.

Most projects that I know that use typebridge forget to handle these return.

I recently encountered a bug related to this and I implemented a retry strategy

export const putEventsWithRetry = async (
  eventBus: Bus,
  events: BusPutEvent[],
  retryCount = 0,
): Promise<void> => {
  const { FailedEntryCount, Entries } = await eventBus.put(events);
  if (FailedEntryCount !== undefined && FailedEntryCount > 0) {
    if (retryCount > 10) {
      throw new Error('Failed to put events in Bus after 10 retries');
    }
    console.warn(`${FailedEntryCount} events failed to be put in Bus:`);
    throwIfNil(Entries, 'Entries should not be undefined if FailedEntryCount is greater than 0');
    const failedEvents = Entries.reduce<BusPutEvent[]>(
      (failedEventsAggregator, { ErrorCode, ErrorMessage }, index) => {
        if (ErrorCode === undefined) {
          return failedEventsAggregator;
        }

        console.warn(`EventBus error: ${ErrorCode} ${ErrorMessage ?? ''}`);
        const failedEvent = events[index];
        throwIfNil(failedEvent, 'Failed event should not be undefined');

        return [...failedEventsAggregator, failedEvent];
      },
      [],
    );
    await putEventsWithRetry(eventBus, failedEvents, retryCount + 1);
  }
};

Can this be the default behavior of Bus.put? and rename the current method put into putWithoutRetrywhcih could be useful to handle retry externaly (for exemple with a step function)

@fredericbarthelet
Copy link
Owner

Hi @CorentinDoue and thanks for the proposal and the code snippet :)

I have a few concerns regarding your proposed feature :

  • Not all errors should be retried - especially, all client side errors shall never be retried with the exact same payload. I'll not be advocating for a retry-everything-all-the-time default implementation.
  • Errors that should be retried are often due to service unavailability (in this case Eventbridge) or throttling/rate limiting issue. In the event such error occurs, exponential backoff and jitter are to be implemented to avoid putting even more stress on an already overwhelmed service

What I'd be willing to add to this package on the other hand to facilitate retry is a new exported function that serve as a middleware that can be added to the Eventbridge client in order to throw a retryable error and leverage existing SDK retry implementation to call again putEvents with a payload only containing original errors. This implementation would still require developper to configure their EventBridge client with the new capability, but it would be much less prone to error, as all existing AWS API behave as such. What do you think of such alternative ?

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

No branches or pull requests

2 participants