-
Notifications
You must be signed in to change notification settings - Fork 1
RabbitMQ: automatic reconnect with topology re-assert on connection/channel loss #120
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import amqplib from "amqplib" | ||
| import { afterAll, beforeEach, describe, expect, mock, spyOn, test } from "bun:test" | ||
| import { RabbitConnection, jitteredDelay } from "./connection" | ||
|
|
||
| describe("jitteredDelay", () => { | ||
| test("returns a value between 50% and 100% of the exponential delay", () => { | ||
| for (let attempt = 0; attempt < 5; attempt++) { | ||
| const delay = jitteredDelay(attempt) | ||
| const exponential = Math.min(1000 * 2 ** attempt, 30_000) | ||
| expect(delay).toBeGreaterThanOrEqual(exponential * 0.5) | ||
| expect(delay).toBeLessThanOrEqual(exponential) | ||
| } | ||
| }) | ||
|
|
||
| test("caps at MAX_RETRY_DELAY_MS", () => { | ||
| const delay = jitteredDelay(100) | ||
| expect(delay).toBeLessThanOrEqual(30_000) | ||
| }) | ||
| }) | ||
|
|
||
| describe("RabbitConnection", () => { | ||
| const mockChannel = { on: mock(() => {}) } | ||
| const mockCreateChannel = mock(() => Promise.resolve(mockChannel)) | ||
|
|
||
| function makeMockConn(onHandler?: (event: string, handler: unknown) => void) { | ||
| return { | ||
| createChannel: mockCreateChannel, | ||
| on: mock(onHandler ?? (() => {})), | ||
| close: mock(() => {}) | ||
| } | ||
| } | ||
|
|
||
| const connectSpy = spyOn(amqplib, "connect") | ||
|
|
||
| beforeEach(() => { | ||
| connectSpy.mockReset() | ||
| mockCreateChannel.mockClear() | ||
| mockCreateChannel.mockResolvedValue(mockChannel) | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| connectSpy.mockRestore() | ||
| }) | ||
|
|
||
| test("createChannel connects and returns a channel", async () => { | ||
| connectSpy.mockResolvedValueOnce(makeMockConn() as never) | ||
|
|
||
| const rabbit = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const channel = await rabbit.createChannel() | ||
|
|
||
| expect(connectSpy).toHaveBeenCalledTimes(1) | ||
| expect(channel).toBe(mockChannel as never) | ||
| }) | ||
|
|
||
| test("concurrent createChannel calls share a single connect attempt", async () => { | ||
| connectSpy.mockResolvedValueOnce(makeMockConn() as never) | ||
|
|
||
| const rabbit = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const [ch1, ch2] = await Promise.all([rabbit.createChannel(), rabbit.createChannel()]) | ||
|
|
||
| expect(connectSpy).toHaveBeenCalledTimes(1) | ||
| expect(ch1).toBe(mockChannel as never) | ||
| expect(ch2).toBe(mockChannel as never) | ||
| }) | ||
|
|
||
| test("reconnects automatically after connection close event", async () => { | ||
| let closeHandler: (() => void) | undefined | ||
| const firstConn = makeMockConn((event, handler) => { | ||
| if (event === "close") closeHandler = handler as () => void | ||
| }) | ||
| const secondConn = makeMockConn() | ||
|
|
||
| connectSpy | ||
| .mockResolvedValueOnce(firstConn as never) | ||
| .mockResolvedValueOnce(secondConn as never) | ||
|
|
||
| const rabbit = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| await rabbit.createChannel() | ||
| expect(connectSpy).toHaveBeenCalledTimes(1) | ||
|
|
||
| // Simulate connection drop | ||
| closeHandler?.() | ||
|
|
||
| // Wait for async reconnect to initiate | ||
| await new Promise(resolve => setTimeout(resolve, 0)) | ||
|
|
||
| // The reconnect promise is now in-flight; await createChannel which waits for it | ||
| await rabbit.createChannel() | ||
| expect(connectSpy).toHaveBeenCalledTimes(2) | ||
| }) | ||
|
|
||
| test("$disconnect sets destroyed flag preventing future reconnects", async () => { | ||
| let closeHandler: (() => void) | undefined | ||
| const conn = makeMockConn((event, handler) => { | ||
| if (event === "close") closeHandler = handler as () => void | ||
| }) | ||
| connectSpy.mockResolvedValueOnce(conn as never) | ||
|
|
||
| const rabbit = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| await rabbit.createChannel() | ||
|
|
||
| rabbit.$disconnect() | ||
| closeHandler?.() | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)) | ||
|
|
||
| // No additional connect call should have been made after destroy | ||
| expect(connectSpy).toHaveBeenCalledTimes(1) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,42 +1,101 @@ | ||||||||||||||||||||||||||||||||||||
| import { Logger } from "@/lib/utils/logging" | ||||||||||||||||||||||||||||||||||||
| import amqp from "amqplib" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const logger = new Logger("RABBITMQ") | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const INITIAL_RETRY_DELAY_MS = parseInt(process.env.RABBITMQ_RETRY_INITIAL_DELAY_MS ?? "1000") | ||||||||||||||||||||||||||||||||||||
| const MAX_RETRY_DELAY_MS = parseInt(process.env.RABBITMQ_RETRY_MAX_DELAY_MS ?? "30000") | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export function jitteredDelay(attempt: number): number { | ||||||||||||||||||||||||||||||||||||
| const exponential = Math.min(INITIAL_RETRY_DELAY_MS * 2 ** attempt, MAX_RETRY_DELAY_MS) | ||||||||||||||||||||||||||||||||||||
| return exponential * (0.5 + 0.5 * Math.random()) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export class RabbitConnection { | ||||||||||||||||||||||||||||||||||||
| private user: string | ||||||||||||||||||||||||||||||||||||
| private password: string | ||||||||||||||||||||||||||||||||||||
| private port: number | ||||||||||||||||||||||||||||||||||||
| private isReady = false | ||||||||||||||||||||||||||||||||||||
| private isConnecting = false | ||||||||||||||||||||||||||||||||||||
| private conn: Promise<amqp.Connection | null> = Promise.resolve(null) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| constructor(args: { user: string; password: string; port: string | number }) { | ||||||||||||||||||||||||||||||||||||
| private readonly user: string | ||||||||||||||||||||||||||||||||||||
| private readonly password: string | ||||||||||||||||||||||||||||||||||||
| private readonly port: number | ||||||||||||||||||||||||||||||||||||
| private readonly heartbeat: number | ||||||||||||||||||||||||||||||||||||
| private conn: amqp.Connection | null = null | ||||||||||||||||||||||||||||||||||||
| private connectPromise: Promise<void> | null = null | ||||||||||||||||||||||||||||||||||||
| private destroyed = false | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| constructor(args: { | ||||||||||||||||||||||||||||||||||||
| user: string | ||||||||||||||||||||||||||||||||||||
| password: string | ||||||||||||||||||||||||||||||||||||
| port: string | number | ||||||||||||||||||||||||||||||||||||
| heartbeat?: number | ||||||||||||||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||||||||||||||
| this.user = args.user | ||||||||||||||||||||||||||||||||||||
| this.password = args.password | ||||||||||||||||||||||||||||||||||||
| this.port = parseInt(args.port.toString()) | ||||||||||||||||||||||||||||||||||||
| this.heartbeat = args.heartbeat ?? parseInt(process.env.RABBITMQ_HEARTBEAT ?? "60") | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| this.heartbeat = args.heartbeat ?? parseInt(process.env.RABBITMQ_HEARTBEAT ?? "60") | |
| const envHeartbeatStr = process.env.RABBITMQ_HEARTBEAT | |
| const envHeartbeat = parseInt(envHeartbeatStr ?? "60", 10) | |
| const heartbeat = args.heartbeat ?? envHeartbeat | |
| if (!Number.isFinite(heartbeat) || heartbeat <= 0) { | |
| const providedValue = | |
| args.heartbeat !== undefined ? String(args.heartbeat) : envHeartbeatStr ?? "60" | |
| throw new Error( | |
| `Invalid RabbitMQ heartbeat value "${providedValue}". It must be a positive number.` | |
| ) | |
| } | |
| this.heartbeat = heartbeat |
Copilot
AI
Feb 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition in connection handling: If the connection is lost immediately after connectPromise completes (line 88) but before this.conn is read (line 90), the handleConnectionLoss event handler could set this.conn to null, causing the check at line 91 to throw "Failed to connect to RabbitMQ" even though reconnection is being initiated.
This is a narrow race window, but consider capturing the connection reference immediately after the await completes to avoid this issue, or add additional null checking with appropriate error messaging.
Copilot
AI
Feb 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The $disconnect() method calls this.conn?.close() which is non-blocking, but it doesn't wait for the close to complete or handle potential errors during close. If close() throws or if there are in-flight operations, they may not be handled gracefully.
Consider using await this.conn?.close() if the close operation is async, or wrapping in try-catch to handle potential errors during disconnection. This is especially important for graceful shutdown scenarios.
| $disconnect(): void { | |
| this.destroyed = true | |
| this.conn?.close() | |
| async $disconnect(): Promise<void> { | |
| this.destroyed = true | |
| if (this.conn) { | |
| try { | |
| await this.conn.close() | |
| } catch (err) { | |
| logger.warn( | |
| "RABBITMQ_DISCONNECT_FAILED", | |
| err instanceof Error ? err : new Error(String(err)) | |
| ) | |
| } finally { | |
| this.conn = null | |
| } | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import amqplib from "amqplib" | ||
| import { afterAll, beforeEach, describe, expect, mock, spyOn, test } from "bun:test" | ||
| import { RabbitConnection } from "./connection" | ||
| import { RabbitQueue } from "./queue" | ||
|
|
||
| describe("RabbitQueue", () => { | ||
| const mockAssertQueue = mock(() => | ||
| Promise.resolve({ queue: "test_queue", messageCount: 0, consumerCount: 0 }) | ||
| ) | ||
| const mockSendToQueue = mock(() => true) | ||
|
|
||
| function makeMockChannel(onHandler?: (event: string, handler: unknown) => void) { | ||
| return { | ||
| assertQueue: mockAssertQueue, | ||
| sendToQueue: mockSendToQueue, | ||
| on: mock(onHandler ?? (() => {})) | ||
| } | ||
| } | ||
|
|
||
| const mockCreateChannel = mock(() => Promise.resolve(makeMockChannel())) | ||
| const mockConn = { | ||
| createChannel: mockCreateChannel, | ||
| on: mock(() => {}), | ||
| close: mock(() => {}) | ||
| } | ||
|
|
||
| const connectSpy = spyOn(amqplib, "connect") | ||
|
|
||
| beforeEach(() => { | ||
| connectSpy.mockReset() | ||
| connectSpy.mockResolvedValue(mockConn as never) | ||
| mockAssertQueue.mockClear() | ||
| mockSendToQueue.mockClear() | ||
| mockCreateChannel.mockClear() | ||
| mockCreateChannel.mockResolvedValue(makeMockChannel()) | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| connectSpy.mockRestore() | ||
| }) | ||
|
|
||
| test("asserts queue topology when channel is first created", async () => { | ||
| const connection = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const queue = new RabbitQueue<number>({ queueName: "test_queue", connection }) | ||
|
|
||
| await queue.send(1) | ||
|
|
||
| expect(mockAssertQueue).toHaveBeenCalledTimes(1) | ||
| expect(mockAssertQueue).toHaveBeenCalledWith("test_queue", { durable: true }) | ||
| }) | ||
|
|
||
| test("sends message after topology assertion", async () => { | ||
| const connection = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const queue = new RabbitQueue<number>({ queueName: "test_queue", connection }) | ||
|
|
||
| await queue.send(42) | ||
|
|
||
| expect(mockSendToQueue).toHaveBeenCalledTimes(1) | ||
| expect(mockSendToQueue).toHaveBeenCalledWith( | ||
| "test_queue", | ||
| Buffer.from("42"), | ||
| expect.objectContaining({ contentType: "text/plain" }) | ||
| ) | ||
| }) | ||
|
|
||
| test("reuses channel for multiple sends without re-asserting", async () => { | ||
| const connection = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const queue = new RabbitQueue<number>({ queueName: "test_queue", connection }) | ||
|
|
||
| await queue.send(1) | ||
| await queue.send(2) | ||
| await queue.send(3) | ||
|
|
||
| expect(mockAssertQueue).toHaveBeenCalledTimes(1) | ||
| expect(mockSendToQueue).toHaveBeenCalledTimes(3) | ||
| }) | ||
|
|
||
| test("re-asserts queue topology after channel loss", async () => { | ||
| let closeHandler: (() => void) | undefined | ||
| mockCreateChannel.mockResolvedValue( | ||
| makeMockChannel((event, handler) => { | ||
| if (event === "close") closeHandler = handler as () => void | ||
| }) | ||
| ) | ||
|
|
||
| const connection = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const queue = new RabbitQueue<number>({ queueName: "test_queue", connection }) | ||
|
|
||
| await queue.send(1) | ||
| expect(mockAssertQueue).toHaveBeenCalledTimes(1) | ||
|
|
||
| // Simulate channel close | ||
| closeHandler?.() | ||
|
|
||
| await queue.send(2) | ||
| expect(mockAssertQueue).toHaveBeenCalledTimes(2) | ||
| expect(mockSendToQueue).toHaveBeenCalledTimes(2) | ||
| }) | ||
|
|
||
| test("sends JSON objects", async () => { | ||
| const connection = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const queue = new RabbitQueue<{ id: number }>({ queueName: "test_queue", connection }) | ||
|
|
||
| await queue.sendJson({ id: 99 }) | ||
|
|
||
| expect(mockSendToQueue).toHaveBeenCalledWith( | ||
| "test_queue", | ||
| Buffer.from(JSON.stringify({ id: 99 })), | ||
| expect.objectContaining({ contentType: "application/json" }) | ||
| ) | ||
| }) | ||
|
|
||
| test("concurrent sends share a single channel creation", async () => { | ||
| const connection = new RabbitConnection({ user: "guest", password: "guest", port: 5672 }) | ||
| const queue = new RabbitQueue<number>({ queueName: "test_queue", connection }) | ||
|
|
||
| await Promise.all([queue.send(1), queue.send(2), queue.send(3)]) | ||
|
|
||
| expect(mockAssertQueue).toHaveBeenCalledTimes(1) | ||
| expect(mockSendToQueue).toHaveBeenCalledTimes(3) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing input validation for environment variables: The
parseInt()calls forRABBITMQ_RETRY_INITIAL_DELAY_MSandRABBITMQ_RETRY_MAX_DELAY_MSdon't validate that the parsed values are valid numbers. If the environment variable contains non-numeric values,parseInt()will returnNaN, which will cause unexpected behavior in thejitteredDelayfunction (resulting inNaNdelays).Consider adding validation to ensure these values are positive numbers, or at minimum check for
isNaN()and provide a fallback or throw a clear error message.