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

When close() is called on a link that got closed due to external factors, the listeners are not cleared #30

Closed
ramya-rao-a opened this issue Mar 8, 2019 · 5 comments

Comments

@ramya-rao-a
Copy link
Collaborator

ramya-rao-a commented Mar 8, 2019

@amarzavery,

Say a link gets closed due to external factors (one way is to disable internet connection in the midst of receiving messages) i.e. no-one called link.close(), but link.isOpen() returns false.
What is the suggested way for consumers of rhea-promise to clean up the link?

link.close() clears only the close and error event listeners that were attached inside the close() function, but not the listeners for all other events that were attached in the _initializeEventListeners function.

Calling link.remove() at this point does clear the listeners, but I am not sure if this is the right way to go about it.

I would expect link.close() to do the right thing here.

cc @ShivangiReja

@ShivangiReja
Copy link
Contributor

ShivangiReja commented Mar 9, 2019

Sample code for the following description:

import {
  Connection, EventContext, ConnectionOptions, Receiver, ReceiverEvents
} from "rhea-promise";
import { ConnectionEvents } from "rhea";

const host = " ";
const username = "";
const password = "";
const port = 5671;
const receiverAddress = "";

async function main(): Promise<void> {
  const connectionOptions: ConnectionOptions = {
    transport: "tls",
    host: host,
    hostname: host,
    username: username,
    password: password,
    port: port,
    reconnect: false
  };

  const connection: Connection = new Connection(connectionOptions);
  await connection.open();

  const receiver: Receiver = await connection.createReceiver({
    name: "receiver-1",
    source: {
      address: receiverAddress
    },
  });

  receiver.on(ReceiverEvents.message, (context: EventContext) => {
    console.log("Received message: %O", context.message);
  });

  receiver.on(ReceiverEvents.receiverError, (context: EventContext) => {
    if (context.receiver && context.receiver.error) {
      console.log(">>>>> An error occurred for receiver: %O.", context.receiver.error);
    }
  });

  const disconnected: any = async (context: EventContext) => {
    console.log("Disconnected Event Fired");
    const connectionError = context.connection && context.connection.error
      ? context.connection.error
      : context.error;
    if (connectionError) {
      console.log("Error occurred on the amqp connection: %O", connectionError);
    }

    console.log(`Reciever is open?: ${receiver.isOpen()}`);
    console.log(`ListenerCount for message event: ${receiver.listenerCount(ReceiverEvents.message)}`);
    console.log(`ListenerCount for error event: ${receiver.listenerCount(ReceiverEvents.receiverError)}`);
    await receiver.close();
    console.log(`After close ListenerCount for message event: ${receiver.listenerCount(ReceiverEvents.message)}`);
    console.log(`After close ListenerCount for error event: ${receiver.listenerCount(ReceiverEvents.receiverError)}`);

  };

  connection.on(ConnectionEvents.disconnected, disconnected);
}

main().catch((err) => console.log(err));

@ramya-rao-a
Copy link
Collaborator Author

While using the above sample code to receive messages, disable the network connection. You will see the handler for the disconnected event being called and the console logs showing the below

  • Receiver is not open
  • There is 1 listener for the message and error event each
  • These listeners remain even after receiver.close() is called.

@ramya-rao-a
Copy link
Collaborator Author

Related issue in rhea: amqp/rhea#205

@ramya-rao-a
Copy link
Collaborator Author

As per discussions in amqp/rhea#205, it is concluded that it is the consumer's responsibility to call remove on the links.

Keeping this issue open to track the need for removeAllSessions function on the connection object which will call the remove_all_sessions function on rhea's connection object thus allowing the consumer to clear things at one go.

cc @ShivangiReja

@ramya-rao-a
Copy link
Collaborator Author

ramya-rao-a commented Mar 19, 2019

this is fixed with #34 and #33 and released in version 0.1.14

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