Skip to content

Conversation

@JulienZD
Copy link
Contributor

@JulienZD JulienZD commented Sep 5, 2025

Summary

Fixes #202.

I also took the liberty to fix the APMQ typos in the debug statements.

Other Information

I tried to write a test for this, but couldn't get anything working that really verified that the issue was fixed. The test just times out...

Here's the test code I attempted with:

Attempt one

  it("unregisters event listeners when amqp connection is closed", async () => {
    const original = { test: "data" };

    let count = 0;

    const incrementCount = () => {
      count++;
    };

    app2.service("todo").on("created", incrementCount);
    onCreated(app2);

    app2.sync.connection.reconnect();

    // Wait for the connection to be back
    await new Promise((resolve) => {
      app2.sync.connection.once("connect", resolve);
    });

    await app1.service("todo").create(original);

    assert.strictEqual(count, 1);

    app2.service("todo").off("created", incrementCount);
  });

Attempt two

  it("unregisters event listeners when amqp connection is closed", async () => {
    const original = { test: "data" };

    let count = 0;

    const incrementCount = () => {
      count++;
    };

    app2.service("todo").on("created", incrementCount);

    app2.sync.connection.close();

    await app2.sync.connection.connect();

    await app1.service("todo").create(original);

    assert.strictEqual(count, 1);

    app2.service("todo").off("created", incrementCount);
  });

@daffl daffl merged commit b67e28d into feathersjs:main Sep 6, 2025
2 checks passed
@daffl
Copy link
Member

daffl commented Sep 6, 2025

Thanks! Released in v3.1.1

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.

AMQP reconnections don't clean up sync-out event handlers, causing memory leaks

2 participants