From 446d19dd3d44cf01a5b41bf2f12811c687fc8004 Mon Sep 17 00:00:00 2001 From: Matthew Petro Date: Sun, 14 Apr 2024 00:24:51 -0700 Subject: [PATCH] Better error handling for irrigation events module --- .../irrigation-events.controller.ts | 4 +- .../irrigation-events.service.spec.ts | 115 ++++++++---------- .../irrigation-events.service.ts | 52 ++++++-- 3 files changed, 92 insertions(+), 79 deletions(-) diff --git a/src/irrigation-events/irrigation-events.controller.ts b/src/irrigation-events/irrigation-events.controller.ts index 0190ebb..8f11b34 100644 --- a/src/irrigation-events/irrigation-events.controller.ts +++ b/src/irrigation-events/irrigation-events.controller.ts @@ -33,8 +33,6 @@ const irrigationEventsToDeviceEvents = (irrigationEvents: IrrigationEvent[]): De return deviceEvents } -// TODO: add error handling in this controller - @Controller('irrigation-events') export class IrrigationEventsController { constructor( @@ -48,7 +46,7 @@ export class IrrigationEventsController { new ValidationPipe({ transform: true, whitelist: true, transformOptions: { enableImplicitConversion: true } }) ) async create(@Body('content') makerApiEventContent: MakerApiEventDto) { - this.irrigationEventsService.insertIrrigationEvent(makerApiEventContent) + this.irrigationEventsService.createIrrigationEvent(makerApiEventContent) } @Get() diff --git a/src/irrigation-events/irrigation-events.service.spec.ts b/src/irrigation-events/irrigation-events.service.spec.ts index 40579ea..a9f182c 100644 --- a/src/irrigation-events/irrigation-events.service.spec.ts +++ b/src/irrigation-events/irrigation-events.service.spec.ts @@ -28,6 +28,7 @@ jest.mock('nano', () => { import { IrrigationEventsService } from '@/irrigation-events/irrigation-events.service' import { DatabaseModule } from '@/database/database.module' +import { HttpException } from '@nestjs/common' const iso8601Regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z$/ @@ -68,10 +69,35 @@ describe('IrrigationEventsService', () => { state: mockIrrigationEvent.value, deviceId: mockIrrigationEvent.deviceId, } - await service.insertIrrigationEvent(mockIrrigationEvent) + mockInsert.mockResolvedValue({ ok: true }) + await service.createIrrigationEvent(mockIrrigationEvent) expect(mockInsert).toHaveBeenCalledWith(mockDocument) }) + it('should throw an exception if the database cannot insert an irrigation event', async () => { + const mockIrrigationEvent: MakerApiEventDto = { + name: 'Name Field', + displayName: 'Display Name Field', + value: DeviceState.ON, + deviceId: 42, + } + mockInsert.mockResolvedValue({ ok: false }) + await expect(service.createIrrigationEvent(mockIrrigationEvent)).rejects.toThrow(HttpException) + expect(mockInsert).toHaveBeenCalled() + }) + + it('should throw an exception if the database throws an error', async () => { + const mockIrrigationEvent: MakerApiEventDto = { + name: 'Name Field', + displayName: 'Display Name Field', + value: DeviceState.ON, + deviceId: 42, + } + mockInsert.mockRejectedValue(new Error('Database error')) + await expect(service.createIrrigationEvent(mockIrrigationEvent)).rejects.toThrow(HttpException) + expect(mockInsert).toHaveBeenCalled() + }) + it('should get irrigation events', async () => { const mockDbDocs: IrrigationEventDocument[] = [ { @@ -105,24 +131,7 @@ describe('IrrigationEventsService', () => { const mockEndTimestamp = '2024-01-02T00:00:00.000Z' mockFind.mockResolvedValue({ docs: mockDbDocs }) const result = await service.getIrrigationEvents(mockStartTimestamp, mockEndTimestamp) - expect(mockFind).toHaveBeenCalledWith({ - selector: { - $and: [ - { - _id: { - $gt: mockStartTimestamp, - }, - }, - { - _id: { - $lt: mockEndTimestamp, - }, - }, - ], - }, - sort: [{ _id: 'asc' }], - limit: 10000, - }) + expect(mockFind).toHaveBeenCalled() expect(result).toEqual(mockResponse) }) @@ -133,6 +142,14 @@ describe('IrrigationEventsService', () => { expect(result).toEqual([]) }) + it('should throw an exception if the database throws an error while getting irrigation events', async () => { + mockFind.mockRejectedValue(new Error('Database error')) + await expect(service.getIrrigationEvents('2024-01-01T00:00:00.000Z', '2024-01-02T00:00:00.000Z')).rejects.toThrow( + HttpException + ) + expect(mockFind).toHaveBeenCalled() + }) + it('should get irrigation events before start', async () => { const mockDbDocs: IrrigationEventDocument[] = [ { @@ -166,28 +183,7 @@ describe('IrrigationEventsService', () => { const mockDeviceId = 42 mockFind.mockResolvedValue({ docs: mockDbDocs }) const result = await service.getEventsBeforeStart(mockStartTimestamp, mockDeviceId) - expect(mockFind).toHaveBeenCalledWith({ - selector: { - $and: [ - { - _id: { - $lt: mockStartTimestamp, - }, - }, - { - deviceId: { - $eq: mockDeviceId, - }, - }, - ], - }, - limit: 2, - sort: [ - { - _id: 'desc', - }, - ], - }) + expect(mockFind).toHaveBeenCalled() expect(result).toEqual(mockResponse) }) @@ -198,6 +194,13 @@ describe('IrrigationEventsService', () => { expect(result).toEqual([]) }) + it('should get an empty array of irrigation events before start if the databse throws an error', async () => { + mockFind.mockRejectedValue(new Error('Database error')) + const result = await service.getEventsBeforeStart('2024-01-01T00:00:00.000Z', 42) + expect(mockFind).toHaveBeenCalled() + expect(result).toEqual([]) + }) + it('should get irrigation events after end', async () => { const mockDbDocs: IrrigationEventDocument[] = [ { @@ -231,31 +234,17 @@ describe('IrrigationEventsService', () => { const mockDeviceId = 42 mockFind.mockResolvedValue({ docs: mockDbDocs }) const result = await service.getEventsAfterEnd(mockEndTimestamp, mockDeviceId) - expect(mockFind).toHaveBeenCalledWith({ - selector: { - $and: [ - { - _id: { - $gt: mockEndTimestamp, - }, - }, - { - deviceId: { - $eq: mockDeviceId, - }, - }, - ], - }, - limit: 2, - sort: [ - { - _id: 'asc', - }, - ], - }) + expect(mockFind).toHaveBeenCalled() expect(result).toEqual(mockResponse) }) + it('should get an empty array of irrigation events after end if the databse throws an error', async () => { + mockFind.mockRejectedValue(new Error('Database error')) + const result = await service.getEventsAfterEnd('2024-01-01T00:00:00.000Z', 42) + expect(mockFind).toHaveBeenCalled() + expect(result).toEqual([]) + }) + it('should get an empty array of irrigation events after end', async () => { mockFind.mockResolvedValue({ docs: [] }) const result = await service.getEventsAfterEnd('2024-01-01T00:00:00.000Z', 42) diff --git a/src/irrigation-events/irrigation-events.service.ts b/src/irrigation-events/irrigation-events.service.ts index c397545..ed03c5c 100644 --- a/src/irrigation-events/irrigation-events.service.ts +++ b/src/irrigation-events/irrigation-events.service.ts @@ -1,4 +1,4 @@ -import { Injectable, OnModuleInit } from '@nestjs/common' +import { HttpException, HttpStatus, Injectable, Logger, OnModuleInit } from '@nestjs/common' import Nano, { DocumentScope } from 'nano' import { ConfigService } from '@nestjs/config' import EnvironmentVariables from '@/environment-variables' @@ -25,10 +25,9 @@ const dbDocumentToIrrigationEvent = ({ _id, deviceName, deviceId, state }: Irrig state: state, }) as IrrigationEvent -// TODO: add error handling in this service - @Injectable() export class IrrigationEventsService implements OnModuleInit { + private readonly logger = new Logger(IrrigationEventsService.name) private db: DocumentScope public constructor( @@ -47,24 +46,51 @@ export class IrrigationEventsService implements OnModuleInit { return result.docs } - public async insertIrrigationEvent(irrigationEvent: MakerApiEventDto) { - await this.db.insert(makerEventToIrrigationEvent(irrigationEvent)) + public async createIrrigationEvent(irrigationEvent: MakerApiEventDto) { + try { + const result = await this.db.insert(makerEventToIrrigationEvent(irrigationEvent)) + if (!result.ok) { + this.logger.error('Failed to create irrigation event', result) + throw new HttpException('Failed to create irrigation event', HttpStatus.INTERNAL_SERVER_ERROR) + } + } catch (error) { + if (error instanceof HttpException) { + throw error + } + this.logger.error('Failed to create irrigation event', error) + throw new HttpException('Failed to create irrigation event', HttpStatus.INTERNAL_SERVER_ERROR) + } } public async getIrrigationEvents(startTimestamp: string, endTimestamp: string) { - const dbDocuments = await this.executeQuery( - queryBuilders.irrigationEventsQueryBuilder(startTimestamp, endTimestamp) - ) - return dbDocuments.map(dbDocumentToIrrigationEvent) + try { + const dbDocuments = await this.executeQuery( + queryBuilders.irrigationEventsQueryBuilder(startTimestamp, endTimestamp) + ) + return dbDocuments.map(dbDocumentToIrrigationEvent) + } catch (error) { + this.logger.error('Failed to retrieve irrigation events', error) + throw new HttpException('Failed to retrieve irrigation events', HttpStatus.INTERNAL_SERVER_ERROR) + } } public async getEventsBeforeStart(startTimestamp: string, deviceId: number) { - const dbDocuments = await this.executeQuery(queryBuilders.eventsBeforeStartQueryBuilder(startTimestamp, deviceId)) - return dbDocuments.map(dbDocumentToIrrigationEvent) + try { + const dbDocuments = await this.executeQuery(queryBuilders.eventsBeforeStartQueryBuilder(startTimestamp, deviceId)) + return dbDocuments.map(dbDocumentToIrrigationEvent) + } catch (error) { + this.logger.error('Failed to retreive events before start', error) + } + return [] } public async getEventsAfterEnd(endTimestamp: string, deviceId: number) { - const dbDocuments = await this.executeQuery(queryBuilders.eventsAfterEndQueryBuilder(endTimestamp, deviceId)) - return dbDocuments.map(dbDocumentToIrrigationEvent) + try { + const dbDocuments = await this.executeQuery(queryBuilders.eventsAfterEndQueryBuilder(endTimestamp, deviceId)) + return dbDocuments.map(dbDocumentToIrrigationEvent) + } catch (error) { + this.logger.error('Failed to retrieve events after end', error) + } + return [] } }