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
legovaer · May 26, 2020 · 37ae1e4 · 37ae1e4
1 parent 5b076de
commit 37ae1e4
Show file tree

Hide file tree

Showing 10 changed files with 166 additions and 13 deletions.
diff --git a/src/integrations/bluetooth-classic/bluetooth-classic.health.spec.ts 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
@@ -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
@@ -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
@@ -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<BluetoothClassicConfig> = {
     addresses: ['8d:ad:e3:e2:7a:01', 'f7:6c:e3:10:55:b5'],
     hciDeviceId: 0,
@@ -80,14 +85,16 @@ describe('BluetoothClassicService', () => {
         ClusterModule,
         ScheduleModule.forRoot(),
       ],
-      providers: [BluetoothClassicService],
+      providers: [BluetoothClassicService, BluetoothClassicHealthIndicator],
     })
       .overrideProvider(EntitiesService)
       .useValue(entitiesService)
       .overrideProvider(ClusterService)
       .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
@@ -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
@@ -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
@@ -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>(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
@@ -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<HealthCheckResult> {
+    return this.health.check(this.healthIndicatorService.getIndicators());
   }
 }
diff --git 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
@@ -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', () => {