Skip to content

Commit

Permalink
feat: option to choose between native and worker timers
Browse files Browse the repository at this point in the history
  • Loading branch information
olso-nordsec committed Mar 15, 2024
1 parent 64ee449 commit 9e956aa
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 28 deletions.
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
16 changes: 12 additions & 4 deletions src/lib/PingTimer.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,36 @@
import timers from './timers'
import getTimer from './get-timer'
import type { TimerVariant } from './shared'

export default class PingTimer {
private keepalive: number

private timer: any

private variant: TimerVariant

private checkPing: () => void

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

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

reschedule() {
this.clear()
this.timer = timers.set(() => {
this.timer = getTimer(this.variant).set(() => {
this.checkPing()
// prevent possible race condition where the timer is destroyed on _cleauUp
// and recreated here
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 @@ -2076,9 +2082,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

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

Check warning on line 26 in src/lib/get-timer.ts

View check run for this annotation

Codecov / codecov/patch

src/lib/get-timer.ts#L26

Added line #L26 was not covered by tests
}
case 'worker': {
return workerTimer

Check warning on line 29 in src/lib/get-timer.ts

View check run for this annotation

Codecov / codecov/patch

src/lib/get-timer.ts#L29

Added line #L29 was not covered by tests
}
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

0 comments on commit 9e956aa

Please sign in to comment.