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

feat: provide attempt number to onRetry callback #95

Merged
merged 1 commit into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -514,10 +514,11 @@ const response1 = await retry(handleAll, { maxAttempts: 3 })

An [event emitter](#events) that fires when we retry a call, before any backoff. It's invoked with an object that includes:

- the `delay` we're going to wait before retrying, and;
- the `delay` we're going to wait before retrying,
- the `attempt` number of the upcoming retry, starting at `1`, and;
- either a thrown error like `{ error: someError, delay: number }`, or an errorful result in an object like `{ value: someValue, delay: number }` when using [result filtering](#handleresulttypector-filter--policyorresulttypector-filter).

Useful for telemetry. Returns a dispable instance.
Useful for telemetry. Returns a disposable instance.

```js
const listener = retry.onRetry(reason => console.log('retrying a function call:', reason));
Expand Down Expand Up @@ -557,7 +558,7 @@ listener.dispose();

### `retry.onGiveUp(callback)`

An [event emitter](#events) that fires when we're no longer retrying a call and are giving up. It's invoked with either a thrown error in an object like `{ error: someError }`, or an errorful result in an object like `{ value: someValue }` when using [result filtering](#handleresulttypector-filter--policyorresulttypector-filter). Useful for telemetry. Returns a dispable instance.
An [event emitter](#events) that fires when we're no longer retrying a call and are giving up. It's invoked with either a thrown error in an object like `{ error: someError }`, or an errorful result in an object like `{ value: someValue }` when using [result filtering](#handleresulttypector-filter--policyorresulttypector-filter). Useful for telemetry. Returns a disposable instance.

```js
const listener = retry.onGiveUp(reason => console.log('retrying a function call:', reason));
Expand Down
12 changes: 12 additions & 0 deletions src/RetryPolicy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,16 @@ describe('RetryPolicy', () => {
await expect(policy.execute(s)).to.eventually.be.rejectedWith(MyErrorA);
expect(onGiveUp).to.have.been.calledWith({ error: err });
});

it('provides the attempt to the onRetry callback', async () => {
const s = stub().throws(new MyErrorA());
const attempts: number[] = [];
const policy = retry(handleAll, { maxAttempts: 3 });
policy.onRetry(({ attempt }) => {
attempts.push(attempt);
});

await expect(policy.execute(s)).to.eventually.be.rejectedWith(MyErrorA);
expect(attempts).to.deep.equal([1, 2, 3]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says attempts should start at 0, but it starts at 1 here. I don't feel strongly either way but one of the two should be updated.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe the documentation is just a little bit unclear. I would say "starting at 1" since 0 doesn't actually get emitted in the retry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll adapt the docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

});
});
6 changes: 4 additions & 2 deletions src/RetryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ export class RetryPolicy implements IPolicy<IRetryContext> {
declare readonly _altReturn: never;

private readonly onGiveUpEmitter = new EventEmitter<FailureReason<unknown>>();
private readonly onRetryEmitter = new EventEmitter<FailureReason<unknown> & { delay: number }>();
private readonly onRetryEmitter = new EventEmitter<
FailureReason<unknown> & { delay: number; attempt: number }
>();

/**
* @inheritdoc
Expand Down Expand Up @@ -112,7 +114,7 @@ export class RetryPolicy implements IPolicy<IRetryContext> {
const delayPromise = delay(delayDuration, !!this.options.unref);
// A little sneaky reordering here lets us use Sinon's fake timers
// when we get an emission in our tests.
this.onRetryEmitter.emit({ ...result, delay: delayDuration });
this.onRetryEmitter.emit({ ...result, delay: delayDuration, attempt: retries + 1 });
await delayPromise;
continue;
}
Expand Down