[Event Hubs] Client throws error when closing due to failure in closing the management link

[ramya-rao-a]

Run the below code a few times.

const { EventHubConsumerClient } = require("@azure/event-hubs");

async function main() {
  const client = new EventHubConsumerClient("$Default", connectionString, "my-hub");
  const partitionsIds = await client.getPartitionIds();
  console.log(partitionsIds);

  let subscription;
  await new Promise((resolve, reject) => {
    subscription = client.subscribe(
      "0",
      {
        processEvents: async () => {
          resolve();
        },
        processError: async (err) => {
          reject(err);
        },
      },
      {
        maxWaitTimeInSeconds: 0,
      }
    );
  });
  await subscription.close();

  await client.close().catch((err) => {
    console.log("error from closing the client", err);
  });
  console.log("client closed");
}

main();

Most of the time it would succeed, but once in a while you will see the below error from client.close()

An error occurred while closing the management session: undefined
    at ManagementClient.<anonymous> (/Users/ramyar/Documents/GitRepos/test/node_modules/@azure/event-hubs/dist/index.js:431:23)
    at Generator.throw (<anonymous>)
    at rejected (/Users/ramyar/Documents/GitRepos/test/node_modules/tslib/tslib.js:113:69)
    at process._tickCallback (internal/process/next_tick.js:68:7)

This does not happen if you remove the code for client.getPartitionIds() or change the maxWaitTimeInSeconds to a value greater than 0

On digging deeper, I found the below:

• Closing of the client involves closing of the management link which in turn involves closing the sender, receiver and the session corresponding to the management link.
• It is the closing of the sender that is failing with error undefined. This is coming from the error event that gets fired when we attempt to close the link. See https://github.com/amqp/rhea-promise/blob/b792d847ba5bb1adf7872aa49a6e1b779e46f754/lib/link.ts#L266-L271
• But in this case, the context.session.error is undefined and therefore we see the same getting bubbled up

On console logging all other places in the context that could have the right error, I found that context.sender.error indeed has the error which is

c {
  value:
   [ Typed { type: [TypeDesc], value: 'amqp:not-found' },
     Typed {
       type: [TypeDesc],
       value:
        'The session channel \'0\' cannot be found in connection \'in-connection2462317(ramyar-eh-test)\'.' },
     Typed { type: [TypeDesc], value: null } ] }

This error seem to be coming from the service and is not something that the client or any of its dependencies throw. I might be wrong.

I don't see this when changing the maxWaitTimeInSeconds to a value greater than 0, so it tells me that this must be some kind of race condition, but I fail to see how this can affect the management link

[ramya-rao-a]

Logged amqp/rhea-promise#54 for follow up in rhea-promise

cc @chradek, @richardpark-msft, @HarshaNalluru

[chradek]

Early update:

Still investigating, but did find something else that's interesting. When I run the sample code above and comment out the 1st client.getPartitionIds() call, the process isn't closing. I can see there are 2 timers (a 2 minute and 1 minute timer, both in rhea) and a socket that's open.

[chradek]

I haven't dug too deep into why the above error is happening, but I did notice something odd with the RequestResponseLink.close() implementation:

azure-sdk-for-js/sdk/core/core-amqp/src/requestResponseLink.ts

Lines 237 to 241 in e51a1d4

	async close(): Promise<void> {
	await this.sender.close();
	await this.receiver.close();
	await this.session.close();
	}

Note that we call close on the sender and receiver before closing the session.

rhea-promise accepts an option closeSession and if it is not set will attempt to close the session after the sender is closed. Since we're closing the session ourselves, we should be passing a value of false for closeSession. When I did this, the above error went away.

Now, I'm not yet sure why this causes an error in sender.close since the session should be closed only after the sender is done closing. I would have expected sender.close() to work and possibly receiver.close() to fail instead. I'll need to investigate a little more why the error is being thrown in sender.close so I can be more sure if this change actually fixes the underlying issue or just improves it.

[ramya-rao-a]

rhea-promise accepts an option closeSession and if it is not set will attempt to close the session after the sender is closed

This was done in amqp/rhea-promise#44
But before adding this option, the default behavior was indeed to close the session as well.
This PR provided a way to opt out of it