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.

Sign up for GitHub

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

update: closing MQTT connection on application shutdown #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

update: closing MQTT connection on application shutdown #16

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/mqtt.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { Inject, Injectable } from '@nestjs/common';
import { Inject, Injectable, OnModuleDestroy } from '@nestjs/common';
import { MQTT_CLIENT_INSTANCE } from './mqtt.constants';
import { Client, Packet, IClientPublishOptions, IClientSubscribeOptions, ISubscriptionGrant } from 'mqtt';

@Injectable()
export class MqttService {
export class MqttService implements OnModuleDestroy {
constructor(
@Inject(MQTT_CLIENT_INSTANCE) private readonly client: Client,
) {}

onModuleDestroy() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires adding app.enableShutdownHooks() to main.ts.

IMHO, the proper way to deal with the closing the client(s) is doing it outside of nest-mqtt:

  • nest-mqtt should export (if it does not already does so) the client object;
  • users should add app.enableShutdownHooks() to their main.ts;
  • create a class for OnModuleDestroy();
  • add that class to the providers array in app.module.ts.

I don’t really think there is anything to do in nest-mqtt (unless client object is not yet exported). However, I don’t use net-mqtt anymore, as I needed to many stuff to implement in order to make it work as I need it to work, therefore I re-implemented it within my codebase.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's not exporting the client yet.

I agree that this line is kind of misplaced. Also delegating this responsibility to users is more flexible in case you don't want client#end to be called for whatever reason.

I'll submit a second PR exposing the client object and documenting how to properly close it's connection. This way, whichever approach is decided to be the best can be merged and the other closed.

this.client.end();
}

subscribe(topic: string | string[], opts?: IClientSubscribeOptions): Promise<ISubscriptionGrant[]> {
return new Promise((resolve, reject) => {
this.client.subscribe(topic, opts || null, (err, granted) => {
Expand Down