Add custom reconnect delay strategy option by GiHoon1123 · Pull Request #857 · amqp-node/amqplib

GiHoon1123 · 2026-07-10T08:50:31Z

Summary

Closes #855.

Adds an optional calculateDelay(attempt) recovery setting for custom reconnect delays such as full jitter, decorrelated jitter, or a fixed schedule.

const connection = await amqplib.connect('amqp://localhost', {
  recovery: {
    maxRetries: Infinity,
    calculateDelay(attempt) {
      return Math.min(30000, 100 * 2 ** (attempt - 1));
    },
  },
});

The option works with both the promise and callback APIs.

Also caps the built-in jittered delay at maxDelay. The base is capped at maxDelay / (1 + jitter) rather than maxDelay itself, so the max jitter offset lands exactly on the cap instead of every delay above it collapsing onto the same value.

Behavior

calculateDelay errors (throw, or an invalid return value) now propagate synchronously instead of falling back silently.
maxDelay only bounds the built-in strategy - calculateDelay callers cap their own delay.
Return values must be finite, non-negative numbers; '1000' isn't coerced.
Without calculateDelay, the built-in strategy is used.

Tests

node --test test/recovery.test.js (12 passing), npm run lint, npm run typecheck pass. Also ran npm test (326 passing) against a local RabbitMQ broker.

Allow opt-in recovery to use a caller-supplied `calculateDelay(attempt)` function instead of the built-in exponential-backoff-with-jitter, for strategies like full jitter, decorrelated jitter, or a fixed schedule. If the function throws or returns something other than a finite, non-negative number, amqplib falls back to the built-in strategy for that attempt and reports the problem via a `handler-error` event, rather than failing to reconnect. Closes amqp-node#855

intech · 2026-08-04T10:44:18Z

Thanks for picking this up, @GiHoon1123 — this is exactly the shape I proposed in #855. I maintain an AMQP adapter (Connectum) built on the v2 recovery, which was the original motivation for the request, so a review from the requester's side:

The API matches the proposal. calculateDelay(attempt) is opt-in and 1-based (consistent with the built-in factor ** (attempt - 1)), absence keeps the current behaviour, and normaliseRecoveryOptions accepting it only when it is a function is the right guard. The validation predicate — typeof === 'number' && Number.isFinite(...) && >= 0, no coercion, so '1000' is rejected rather than silently accepted — is the correct strictness, and falling back to the built-in strategy instead of throwing out of an unawaited reconnect callback is the right call for a path that would otherwise take the process down.

Two things:

1. CI has not run. The PR currently reports zero check runs, which looks like the pending-approval gate for a first-time contributor rather than anything wrong with the branch. @cressie176 — could you approve the workflow run so the suite reports? Nothing else here is blocked on the author.

2. Scope of "Closes #855". #855 raised two points, and this PR addresses one of them. The hook lets a caller avoid the cap overshoot by owning the number, but calculateBuiltinDelay is unchanged, so the numeric-knob path still applies maxDelay to the base before jitter and can exceed the cap by the jitter fraction (up to ~2x at jitter: 1). Worth either fixing that here as well, or keeping a tracking issue open so the automatic close of #855 does not drop it.

Happy to help validate: the PR notes the recovery tests use an in-memory model, and we have a testcontainers-based RabbitMQ recovery harness. I can run this branch against a live broker — measuring the actual inter-attempt intervals across real connection drops, plus the fallback-on-throw and fallback-on-invalid paths — and report back, or contribute a broker-level integration test in the style of the existing suite, whichever is more useful to you.

GiHoon1123 · 2026-08-04T13:07:30Z

Good catch. I pushed a follow-up that clamps the built-in delay after jitter and added a deterministic test for the maximum positive jitter case.

The recovery tests, lint, and typecheck pass locally.

intech · 2026-08-04T13:38:25Z

The clamp looks right, and thanks for turning it around so fast — that closes the second half of #855.

One observation about the shape it produces, worth considering before this lands. Clamping after symmetric jitter puts a point mass exactly on the cap: at steady state base has already saturated to maxDelay, so offset > 0 — half the draws at any jitter — now collapses onto maxDelay itself. With the default jitter: 0.2 that means roughly half of all reconnects in a fleet fire at the same instant, which is the opposite of what the jitter is there for. Before the change those draws were spread over (maxDelay, maxDelay × 1.2].

The usual ways to keep the cap without the atom are to shrink the base (min(maxDelay / (1 + jitter), ...)) or to jitter downward only (base × (1 - jitter * Math.random())). Both shift the mean, so it is a real trade-off rather than an obvious win — your call, and the maintainer's. Flagging it because it changes the built-in distribution for every existing user, not only for those who were exceeding the cap.

Either way calculateDelay makes it a non-issue for anyone who cares about the exact shape, which is an argument for landing both in the same release.

cressie176 · 2026-08-10T14:47:02Z

Thanks for the PR @GiHoon1123 and the thoughtful comments @intech. I plan to go through this tonight.

cressie176 · 2026-08-10T21:06:22Z

+      }
+      throw new Error(`calculateDelay must return a finite, non-negative number of milliseconds (got ${String(custom)})`);
+    } catch (err) {
+      setImmediate(() => core.emit('handler-error', toError(err, 'calculateDelay failed'), 'calculateDelay'));


handler-error is for when a custom error handler throws an exception. I don't think we should use it in this instance. I think I would prefer throwing the error synchronously

cressie176 · 2026-08-10T21:09:17Z

+// as an uncaught exception or unhandled rejection and crash the process.
+// Fall back to the built-in strategy and report the problem via
+// `handler-error` unconditionally - a harmless no-op if nobody's listening.
+function resolveDelay(core, recovery, attempt) {


"resolve" in "resolveDelay" implies asynchronous calculation. I might just call it "calculateDelay" and have it delegate to the supplied function or built in.

cressie176 · 2026-08-10T21:09:43Z

+// Fall back to the built-in strategy and report the problem via
+// `handler-error` unconditionally - a harmless no-op if nobody's listening.
+function resolveDelay(core, recovery, attempt) {
+  if (recovery.calculateDelay) {


Prefer guard conditions (immediate return) rather than long lived if and nesting

cressie176 · 2026-08-10T21:42:31Z


 Without `recovery` options, behavior is unchanged.

+### Custom delay strategy


Needs to be clear that the user must implement maxDelay otherwise they might assume the library will enforce it

cressie176 · 2026-08-10T21:45:12Z

  const jitterPart = base * recovery.jitter;
  const offset = jitterPart > 0 ? Math.random() * jitterPart * 2 - jitterPart : 0;
-  return Math.max(0, Math.round(base + offset));
+  return Math.min(recovery.maxDelay, Math.max(0, Math.round(base + offset)));


As @intech pointed out, enforcing maxDelay means the jitter will not be applied once maxDelay has been exceeded. I would prefer to keep the jitter, or capping the base to maxDelay - maxJitter so there is always room for the jitter?

"resolve" implies asynchronous work; this just delegates to the supplied function or the built-in strategy.

No behavior change - inverts the condition to return early for the no-custom-strategy case instead of nesting the rest of the function inside the if branch.

A broken custom calculateDelay (throws, or returns something other than a finite, non-negative number) is a bug in caller-supplied code. Silently falling back to the built-in strategy and reporting it via handler-error hid the problem instead of surfacing it - switch to letting it propagate synchronously so it's caught immediately. Drops the now-unused handler-error fallback path and the core parameter it needed. Updates tests accordingly; removes the test for the old no-listener fallback behavior, which no longer applies.

Callers supplying calculateDelay must enforce their own cap - amqplib doesn't apply maxDelay on top of a custom function's return value.

Capping base at maxDelay and then clamping base+offset put a point mass exactly on maxDelay once base saturated: every positive jitter draw (half of them) collapsed onto the same value, defeating the purpose of jitter - a fleet of clients would reconnect in lockstep at that instant. Capping base at maxDelay / (1 + jitter) instead means the maximum possible offset lands exactly on maxDelay, so the result never exceeds it without a final clamp, and the distribution stays smooth right up to the cap. Adds a regression test with a fixed Math.random() proving the result is no longer collapsed onto maxDelay for a mid-range jitter draw once the base has saturated.

Missed updating these when calculateDelay switched from catching and falling back (via handler-error) to propagating synchronously - the JSDoc still described the old behavior.

GiHoon1123 · 2026-08-11T02:41:55Z

Thanks for the review - pushed changes addressing all of this:

Renamed resolveDelay to calculateDelay, switched to a guard clause.
calculateDelay errors now propagate synchronously instead of falling back via handler-error.
README now says maxDelay only bounds the built-in strategy.
Fixed the jitter point mass @intech flagged - base is capped at maxDelay / (1 + jitter) now instead of maxDelay. Added a regression test for it.

One consequence of the synchronous throw: it happens before _attempt/_timer get updated in _scheduleReconnect, so a broken calculateDelay doesn't just fail one attempt - the connection stops reconnecting entirely after that, with no reconnect-failed or error event. Flagging it in case it's worth addressing here or separately.

intech mentioned this pull request Aug 4, 2026

feat(events-amqp): pluggable recovery backoff hook (backoff(attempt)) Connectum-Framework/connectum#199

Open

6 tasks

fix(recovery): cap jittered delay at maxDelay

0be6876

cressie176 requested changes Aug 10, 2026

View reviewed changes

GiHoon1123 added 6 commits August 11, 2026 09:56

rename resolveDelay to calculateDelay

d32a236

"resolve" implies asynchronous work; this just delegates to the supplied function or the built-in strategy.

use a guard clause in calculateDelay instead of nesting

669b166

No behavior change - inverts the condition to return early for the no-custom-strategy case instead of nesting the rest of the function inside the if branch.

docs: clarify maxDelay only bounds the built-in strategy

947fe31

Callers supplying calculateDelay must enforce their own cap - amqplib doesn't apply maxDelay on top of a custom function's return value.

docs(types): update calculateDelay JSDoc for synchronous-throw behavior

7e7ced9

Missed updating these when calculateDelay switched from catching and falling back (via handler-error) to propagating synchronously - the JSDoc still described the old behavior.

Navigation Menu

Uh oh!

Add custom reconnect delay strategy option - #857

Add custom reconnect delay strategy option#857
GiHoon1123 wants to merge 8 commits into
amqp-node:mainfrom
GiHoon1123:feat/custom-reconnect-delay-strategy

GiHoon1123 commented Jul 10, 2026 •

edited

Loading

Uh oh!

intech commented Aug 4, 2026

Uh oh!

GiHoon1123 commented Aug 4, 2026

Uh oh!

intech commented Aug 4, 2026

Uh oh!

cressie176 commented Aug 10, 2026

Uh oh!

cressie176 Aug 10, 2026

Uh oh!

cressie176 Aug 10, 2026

Uh oh!

cressie176 Aug 10, 2026

Uh oh!

cressie176 Aug 10, 2026

Uh oh!

cressie176 Aug 10, 2026

Uh oh!

GiHoon1123 commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		Without `recovery` options, behavior is unchanged.

		### Custom delay strategy

Uh oh!

Conversation

GiHoon1123 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Behavior

Tests

Uh oh!

intech commented Aug 4, 2026

Uh oh!

GiHoon1123 commented Aug 4, 2026

Uh oh!

intech commented Aug 4, 2026

Uh oh!

cressie176 commented Aug 10, 2026

Uh oh!

cressie176 Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

cressie176 Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

cressie176 Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

cressie176 Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

cressie176 Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

GiHoon1123 commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GiHoon1123 commented Jul 10, 2026 •

edited

Loading