fix(client): Checks if the gateway is already closed before trying to shutdown the gateway

[bulldetektor]

related to #39

[ChrisKujawa]

Please lets not ignore exceptions. Is there a specific reason for that?

[bulldetektor]

Because Dispose() should never throw exceptions, and there's no guarantee that ShutdownAsync won't throw here (in fact there's a fair chance it actually will throw...)

[bulldetektor]

That said; If you'd rather have the exception be thrown here, I can remove the try-catch. The real fix is the guard above that will make sure we're not calling ShotdownAsync() on a ClosedGatewayClient (which is guaranteed to throw an exception)

[ChrisKujawa]

yeah lets remove it

[ChrisKujawa]

Hey @bulldetektor first of all thanks so much for your contribution I appreciate it!

Is there a reason why implemented for the PublishMessageCommand only and how is it related to #39?

[bulldetektor]

The relation to #39 is that handling exceptions in Dispose will get you closer to a graceful shutdown of the client. So I'm not stating that it will close the issue, but it will make the shutdown "more" graceful :)

As for why I only implemented it for the PublishMessageCommand; That's because I'm not familiar enough with the code base to start doing major refactorings, and this was just to patch up the errors that we were experiencing when using the client. I could probably do the same for the rest of the commands, but not sure if you'd appreciate such I large change from a first-time committer on the project :)

[ChrisKujawa]

@bulldetektor ah thanks for the explanation, makes sense thanks. I have overseen that you used the related mark. I thought it was closes.

I assume we could also add an unit test that we can close the client twice.

[bulldetektor]

@Zelldon I removed the try-catch and added a unit test that verifies that Dispose does not throw any exceptions when called multiple times (also checked that the test fails when I remove my fix)

[ChrisKujawa]

Awesome thanks 👍