From 37ae1e4f25a1a9bd2b8193bf4d054d87a0432ec3 Mon Sep 17 00:00:00 2001 From: Heiko Rothe Date: Tue, 26 May 2020 22:05:00 +0200 Subject: [PATCH] feat(bluetooth-classic): monitor command health Too many successive errors from the hcitool commands will now make the room-assistant instance report as unhealthy. Closes #194 --- .../bluetooth-classic.health.spec.ts | 35 ++++++++++++++++ .../bluetooth-classic.health.ts | 40 +++++++++++++++++++ .../bluetooth-classic.module.ts | 6 ++- .../bluetooth-classic.service.spec.ts | 35 +++++++++++++++- .../bluetooth-classic.service.ts | 12 +++++- src/status/health-indicator.service.ts | 17 ++++++++ src/status/status.controller.spec.ts | 9 ++++- src/status/status.controller.ts | 16 +++++++- src/status/status.module.ts | 7 +++- src/status/status.service.spec.ts | 2 +- 10 files changed, 166 insertions(+), 13 deletions(-) create mode 100644 src/integrations/bluetooth-classic/bluetooth-classic.health.spec.ts create mode 100644 src/integrations/bluetooth-classic/bluetooth-classic.health.ts create mode 100644 src/status/health-indicator.service.ts diff --git a/src/integrations/bluetooth-classic/bluetooth-classic.health.spec.ts b/src/integrations/bluetooth-classic/bluetooth-classic.health.spec.ts new file mode 100644 index 0000000..8a59c9f --- /dev/null +++ b/src/integrations/bluetooth-classic/bluetooth-classic.health.spec.ts @@ -0,0 +1,35 @@ +import { BluetoothClassicHealthIndicator } from './bluetooth-classic.health'; +import { HealthCheckError } from '@nestjs/terminus'; + +describe('BluetoothClassicHealthIndicator', () => { + let healthIndicator: BluetoothClassicHealthIndicator; + + beforeEach(() => { + healthIndicator = new BluetoothClassicHealthIndicator(); + }); + + it('should report healthy by default', () => { + const result = healthIndicator.successiveErrorCheck(3); + expect(result['bt_successive_errors'].status).toEqual('up'); + }); + + it('should report unhealthy after meeting the threshold', () => { + healthIndicator.reportError(); + healthIndicator.reportError(); + healthIndicator.reportError(); + + expect(() => healthIndicator.successiveErrorCheck(3)).toThrow( + HealthCheckError + ); + }); + + it('should reset the error count on success', () => { + healthIndicator.reportError(); + healthIndicator.reportError(); + healthIndicator.reportError(); + healthIndicator.reportSuccess(); + + const result = healthIndicator.successiveErrorCheck(3); + expect(result['bt_successive_errors'].status).toEqual('up'); + }); +}); diff --git a/src/integrations/bluetooth-classic/bluetooth-classic.health.ts b/src/integrations/bluetooth-classic/bluetooth-classic.health.ts new file mode 100644 index 0000000..6d28122 --- /dev/null +++ b/src/integrations/bluetooth-classic/bluetooth-classic.health.ts @@ -0,0 +1,40 @@ +import { + HealthCheckError, + HealthIndicator, + HealthIndicatorResult, +} from '@nestjs/terminus'; +import { Injectable, Optional } from '@nestjs/common'; +import { HealthIndicatorService } from '../../status/health-indicator.service'; + +@Injectable() +export class BluetoothClassicHealthIndicator extends HealthIndicator { + private errorsOccurred = 0; + + constructor(@Optional() healthIndicatorService?: HealthIndicatorService) { + super(); + healthIndicatorService?.registerHealthIndicator(async () => + this.successiveErrorCheck(3) + ); + } + + successiveErrorCheck(threshold: number): HealthIndicatorResult { + const isHealthy = this.errorsOccurred < threshold; + const result = this.getStatus(`bt_successive_errors`, isHealthy); + + if (isHealthy) { + return result; + } + throw new HealthCheckError( + 'BT Classic successive error check failed', + result + ); + } + + reportError(): void { + this.errorsOccurred++; + } + + reportSuccess(): void { + this.errorsOccurred = 0; + } +} diff --git a/src/integrations/bluetooth-classic/bluetooth-classic.module.ts b/src/integrations/bluetooth-classic/bluetooth-classic.module.ts index a4be742..37b0ae3 100644 --- a/src/integrations/bluetooth-classic/bluetooth-classic.module.ts +++ b/src/integrations/bluetooth-classic/bluetooth-classic.module.ts @@ -3,14 +3,16 @@ import { BluetoothClassicService } from './bluetooth-classic.service'; import { ConfigModule } from '../../config/config.module'; import { EntitiesModule } from '../../entities/entities.module'; import { ClusterModule } from '../../cluster/cluster.module'; +import { BluetoothClassicHealthIndicator } from './bluetooth-classic.health'; +import { StatusModule } from '../../status/status.module'; @Module({}) export default class BluetoothClassicModule { static forRoot(): DynamicModule { return { module: BluetoothClassicModule, - imports: [ConfigModule, EntitiesModule, ClusterModule], - providers: [BluetoothClassicService], + imports: [ConfigModule, EntitiesModule, ClusterModule, StatusModule], + providers: [BluetoothClassicService, BluetoothClassicHealthIndicator], }; } } diff --git a/src/integrations/bluetooth-classic/bluetooth-classic.service.spec.ts b/src/integrations/bluetooth-classic/bluetooth-classic.service.spec.ts index 61f19a7..4f8f033 100644 --- a/src/integrations/bluetooth-classic/bluetooth-classic.service.spec.ts +++ b/src/integrations/bluetooth-classic/bluetooth-classic.service.spec.ts @@ -17,6 +17,7 @@ import { RoomPresenceDistanceSensor } from '../room-presence/room-presence-dista import KalmanFilter from 'kalmanjs'; import { Switch } from '../../entities/switch'; import { BluetoothClassicConfig } from './bluetooth-classic.config'; +import { BluetoothClassicHealthIndicator } from './bluetooth-classic.health'; import c from 'config'; import { ConfigService } from '../../config/config.service'; import { Device } from './device'; @@ -58,6 +59,10 @@ describe('BluetoothClassicService', () => { error: jest.fn(), warn: jest.fn(), }; + const healthIndicator = { + reportError: jest.fn(), + reportSuccess: jest.fn(), + }; const config: Partial = { addresses: ['8d:ad:e3:e2:7a:01', 'f7:6c:e3:10:55:b5'], hciDeviceId: 0, @@ -80,7 +85,7 @@ describe('BluetoothClassicService', () => { ClusterModule, ScheduleModule.forRoot(), ], - providers: [BluetoothClassicService], + providers: [BluetoothClassicService, BluetoothClassicHealthIndicator], }) .overrideProvider(EntitiesService) .useValue(entitiesService) @@ -88,6 +93,8 @@ describe('BluetoothClassicService', () => { .useValue(clusterService) .overrideProvider(ConfigService) .useValue(configService) + .overrideProvider(BluetoothClassicHealthIndicator) + .useValue(healthIndicator) .compile(); module.useLogger(loggerService); @@ -164,7 +171,7 @@ describe('BluetoothClassicService', () => { expect(service.inquireRssi('08:05:90:ed:3b:60')).resolves.toBeUndefined(); }); - it('should return reset the HCI device if the query took too long', async () => { + it('should reset the HCI device if the query took too long', async () => { mockExec.mockRejectedValue({ signal: 'SIGKILL' }); const result = await service.inquireRssi('08:05:90:ed:3b:60'); @@ -665,4 +672,28 @@ Requesting information ... expect(service.shouldInquire()).toBeTruthy(); }); + + it('should report success to the health indicator when queries are successful', async () => { + mockExec.mockResolvedValue({ stdout: 'RSSI return value: -4' }); + await service.inquireRssi(''); + + expect(healthIndicator.reportSuccess).toHaveBeenCalledTimes(1); + }); + + it('should report an error to the health indicator when queries are unsuccessful', async () => { + mockExec.mockRejectedValue({ message: 'critical error' }); + await service.inquireRssi(''); + + expect(healthIndicator.reportError).toHaveBeenCalledTimes(1); + }); + + it('should not report anything to the health indicator if the device was not reachable', async () => { + mockExec.mockRejectedValue({ + message: 'Could not connect: Input/output error', + }); + await service.inquireRssi(''); + + expect(healthIndicator.reportSuccess).not.toHaveBeenCalled(); + expect(healthIndicator.reportError).not.toHaveBeenCalled(); + }); }); diff --git a/src/integrations/bluetooth-classic/bluetooth-classic.service.ts b/src/integrations/bluetooth-classic/bluetooth-classic.service.ts index 1d7a92c..dfcfb7e 100644 --- a/src/integrations/bluetooth-classic/bluetooth-classic.service.ts +++ b/src/integrations/bluetooth-classic/bluetooth-classic.service.ts @@ -29,6 +29,7 @@ import { Switch } from '../../entities/switch'; import { SwitchConfig } from '../home-assistant/switch-config'; import { DeviceTracker } from '../../entities/device-tracker'; import { RoomPresenceDeviceTrackerProxyHandler } from '../room-presence/room-presence-device-tracker.proxy'; +import { BluetoothClassicHealthIndicator } from './bluetooth-classic.health'; const execPromise = util.promisify(exec); @@ -45,7 +46,8 @@ export class BluetoothClassicService extends KalmanFilterable(Object, 1.4, 1) private readonly configService: ConfigService, private readonly entitiesService: EntitiesService, private readonly clusterService: ClusterService, - private readonly schedulerRegistry: SchedulerRegistry + private readonly schedulerRegistry: SchedulerRegistry, + private readonly healthIndicator: BluetoothClassicHealthIndicator ) { super(); this.config = this.configService.get('bluetoothClassic'); @@ -215,15 +217,21 @@ export class BluetoothClassicService extends KalmanFilterable(Object, 1.4, 1) ); const matches = output.stdout.match(regex); + this.healthIndicator.reportSuccess(); + return matches?.length > 0 ? parseInt(matches[0], 10) : undefined; } catch (e) { if (e.signal === 'SIGKILL') { this.logger.warn( `Query of ${address} took too long, resetting hci${this.config.hciDeviceId}` ); + this.healthIndicator.reportError(); this.resetHciDevice(); - } else { + } else if (e.message?.includes('Input/output error')) { this.logger.debug(e.message); + } else { + this.logger.error(e.message); + this.healthIndicator.reportError(); } return undefined; diff --git a/src/status/health-indicator.service.ts b/src/status/health-indicator.service.ts new file mode 100644 index 0000000..4025ffb --- /dev/null +++ b/src/status/health-indicator.service.ts @@ -0,0 +1,17 @@ +import { HealthIndicatorFunction } from '@nestjs/terminus'; +import { Injectable } from '@nestjs/common'; + +@Injectable() +export class HealthIndicatorService { + private healthIndicators: HealthIndicatorFunction[] = []; + + getIndicators(): HealthIndicatorFunction[] { + return this.healthIndicators; + } + + registerHealthIndicator( + healthIndicatorFunction: HealthIndicatorFunction + ): void { + this.healthIndicators.push(healthIndicatorFunction); + } +} diff --git a/src/status/status.controller.spec.ts b/src/status/status.controller.spec.ts index f1735d6..824619c 100644 --- a/src/status/status.controller.spec.ts +++ b/src/status/status.controller.spec.ts @@ -1,18 +1,23 @@ import { Test, TestingModule } from '@nestjs/testing'; import { StatusController } from './status.controller'; +import { TerminusModule } from '@nestjs/terminus'; +import { HealthIndicatorService } from './health-indicator.service'; describe('Status Controller', () => { let controller: StatusController; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ + imports: [TerminusModule], controllers: [StatusController], + providers: [HealthIndicatorService], }).compile(); controller = module.get(StatusController); }); - it('should return healthy', () => { - expect(controller.health()).toEqual('healthy'); + it('should return healthy', async () => { + const status = await controller.check(); + expect(status.status).toEqual('ok'); }); }); diff --git a/src/status/status.controller.ts b/src/status/status.controller.ts index 5e9f14c..a84c9b1 100644 --- a/src/status/status.controller.ts +++ b/src/status/status.controller.ts @@ -1,9 +1,21 @@ import { Controller, Get } from '@nestjs/common'; +import { + HealthCheck, + HealthCheckResult, + HealthCheckService, +} from '@nestjs/terminus'; +import { HealthIndicatorService } from './health-indicator.service'; @Controller('status') export class StatusController { + constructor( + private health: HealthCheckService, + private healthIndicatorService: HealthIndicatorService + ) {} + @Get() - health(): string { - return 'healthy'; + @HealthCheck() + check(): Promise { + return this.health.check(this.healthIndicatorService.getIndicators()); } } diff --git a/src/status/status.module.ts b/src/status/status.module.ts index 5e620cc..65fd1e4 100644 --- a/src/status/status.module.ts +++ b/src/status/status.module.ts @@ -4,10 +4,13 @@ import { ClusterModule } from '../cluster/cluster.module'; import { EntitiesModule } from '../entities/entities.module'; import { ConfigModule } from '../config/config.module'; import { StatusController } from './status.controller'; +import { TerminusModule } from '@nestjs/terminus'; +import { HealthIndicatorService } from './health-indicator.service'; @Module({ - imports: [ClusterModule, EntitiesModule, ConfigModule], - providers: [StatusService], + imports: [ClusterModule, EntitiesModule, ConfigModule, TerminusModule], + providers: [StatusService, HealthIndicatorService], controllers: [StatusController], + exports: [HealthIndicatorService], }) export class StatusModule {} diff --git a/src/status/status.service.spec.ts b/src/status/status.service.spec.ts index 8ea2b03..f89b825 100644 --- a/src/status/status.service.spec.ts +++ b/src/status/status.service.spec.ts @@ -80,7 +80,7 @@ describe('StatusService', () => { service.onApplicationBootstrap(); - expect(clusterService.on).toHaveBeenCalledTimes(3); + expect(clusterService.on).toHaveBeenCalledTimes(4); }); it('should update the cluster size sensor', () => {