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

feat: timerVariant option to choose between native and worker timers #1818

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ The arguments are:
- `messageIdProvider`: custom messageId provider. when `new UniqueMessageIdProvider()` is set, then non conflict messageId is provided.
- `log`: custom log function. Default uses [debug](https://www.npmjs.com/package/debug) package.
- `manualConnect`: prevents the constructor to call `connect`. In this case after the `mqtt.connect` is called you should call `client.connect` manually.
- `timerVariant`: defaults to `auto`, which tries to determine which timer is most appropriate for you environment, if you're having detection issues, you can set it to `worker` or `native`

In case mqtts (mqtt over tls) is required, the `options` object is passed through to [`tls.connect()`](http://nodejs.org/api/tls.html#tls_tls_connect_options_callback). If using a **self-signed certificate**, set `rejectUnauthorized: false`. However, be cautious as this exposes you to potential man in the middle attacks and isn't recommended for production.

Expand Down
24 changes: 16 additions & 8 deletions src/lib/PingTimer.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,40 @@
import timers from './timers'
import getTimer, { type Timer } from './get-timer'
import type { TimerVariant } from './shared'

export default class PingTimer {
private keepalive: number

private timer: any
private timerId: number

private timer: Timer

private checkPing: () => void

constructor(keepalive: number, checkPing: () => void) {
constructor(
keepalive: number,
checkPing: () => void,
variant: TimerVariant,
) {
this.keepalive = keepalive * 1000
this.checkPing = checkPing
this.timer = getTimer(variant)
this.reschedule()
}

clear() {
if (this.timer) {
timers.clear(this.timer)
this.timer = null
if (this.timerId) {
this.timer.clear(this.timerId)
this.timerId = null
}
}

reschedule() {
this.clear()
this.timer = timers.set(() => {
this.timerId = this.timer.set(() => {
this.checkPing()
// prevent possible race condition where the timer is destroyed on _cleauUp
// and recreated here
if (this.timer) {
if (this.timerId) {
this.reschedule()
}
}, this.keepalive)
Expand Down
18 changes: 14 additions & 4 deletions src/lib/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
GenericCallback,
IStream,
StreamBuilder,
TimerVariant,
VoidCallback,
nextTick,
} from './shared'
Expand All @@ -49,7 +50,7 @@ const setImmediate =
})
}) as typeof globalThis.setImmediate)

const defaultConnectOptions = {
const defaultConnectOptions: IClientOptions = {
keepalive: 60,
reschedulePings: true,
protocolId: 'MQTT',
Expand All @@ -59,6 +60,7 @@ const defaultConnectOptions = {
clean: true,
resubscribe: true,
writeCache: true,
timerVariant: 'auto',
}

export type MqttProtocol =
Expand Down Expand Up @@ -266,6 +268,10 @@ export interface IClientOptions extends ISecureClientOptions {
will?: IConnectPacket['will']
/** see `connect` packet: https://github.com/mqttjs/mqtt-packet/blob/master/types/index.d.ts#L65 */
properties?: IConnectPacket['properties']
/**
* @description 'auto', set to 'native' or 'worker' if you're having issues with 'auto' detection
*/
timerVariant?: TimerVariant
}

export interface IClientPublishOptions {
Expand Down Expand Up @@ -2078,9 +2084,13 @@ export default class MqttClient extends TypedEventEmitter<MqttClientEventCallbac

if (!this.pingTimer && this.options.keepalive) {
this.pingResp = true
this.pingTimer = new PingTimer(this.options.keepalive, () => {
this._checkPing()
})
this.pingTimer = new PingTimer(
this.options.keepalive,
() => {
this._checkPing()
},
this.options.timerVariant,
)
}
}

Expand Down
38 changes: 38 additions & 0 deletions src/lib/get-timer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import isBrowser, { isWebWorker } from './is-browser'
import { clearTimeout as clearT, setTimeout as setT } from 'worker-timers'
import type { TimerVariant } from './shared'

// dont directly assign globals to class props otherwise this throws in web workers: Uncaught TypeError: Illegal invocation
// See: https://stackoverflow.com/questions/9677985/uncaught-typeerror-illegal-invocation-in-chrome

export interface Timer {
set: typeof setT
clear: typeof clearT
}

const workerTimer: Timer = {
set: setT,
clear: clearT,
}

const nativeTimer: Timer = {
set: (func, time) => setTimeout(func, time),
clear: (timerId) => clearTimeout(timerId),
}

const getTimer = (variant: TimerVariant): Timer => {
switch (variant) {
case 'native': {
return nativeTimer
}
case 'worker': {
return workerTimer
}
case 'auto':
default: {
return isBrowser && !isWebWorker ? workerTimer : nativeTimer
}
}
}

export default getTimer
2 changes: 2 additions & 0 deletions src/lib/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export type PacketHandler = (
done?: DoneCallback,
) => void

export type TimerVariant = 'auto' | 'worker' | 'native'

export class ErrorWithReasonCode extends Error {
public code: number

Expand Down
15 changes: 0 additions & 15 deletions src/lib/timers.ts

This file was deleted.

14 changes: 9 additions & 5 deletions test/pingTimer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('PingTimer', () => {
it('should schedule and clear', () => {
const keepalive = 10 // seconds
const cb = spy()
const pingTimer = new PingTimer(keepalive, cb)
const pingTimer = new PingTimer(keepalive, cb, 'auto')

assert.ok(pingTimer['timer'], 'timer should be created automatically')

Expand All @@ -35,10 +35,14 @@ describe('PingTimer', () => {
it('should not re-schedule if timer has been cleared in check ping', () => {
const keepalive = 10 // seconds
const cb = spy()
const pingTimer = new PingTimer(keepalive, () => {
pingTimer.clear()
cb()
})
const pingTimer = new PingTimer(
keepalive,
() => {
pingTimer.clear()
cb()
},
'auto',
)

clock.tick(keepalive * 1000 + 1)
assert.equal(
Expand Down