Skip to content

Commit

Permalink
BC-8308 add warning if an error occurs during healtch check (#5310)
Browse files Browse the repository at this point in the history
  • Loading branch information
Loki-Afro authored Oct 28, 2024
1 parent 3825688 commit 52a9712
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 12 deletions.
13 changes: 12 additions & 1 deletion apps/server/src/core/error/loggable/error.loggable.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NotFound } from '@feathersjs/errors';
import { BadRequestException, HttpStatus, ValidationError } from '@nestjs/common';
import { ApiProperty } from '@nestjs/swagger';
import { ApiValidationError, BusinessError } from '@shared/common';
import { ApiValidationError, BusinessError, ErrorLogMessage } from '@shared/common';
import { PrivacyProtect } from '@shared/controller';
import { ErrorLoggable } from './error.loggable';

Expand Down Expand Up @@ -158,5 +158,16 @@ describe('ErrorLoggable', () => {
expect(message).toEqual(expectedMessage);
});
});

describe('when error is of unknown type', () => {
it('should return ErrorLogMessage with appropriate error type', () => {
const errorLoggable = new ErrorLoggable('something broke');

const message = errorLoggable.getLogMessage() as ErrorLogMessage;
expect(message.data).toBeUndefined();
expect(message.error).toBeInstanceOf(Error);
expect(message.type).toBe('Unhandled or Unknown Error');
});
});
});
});
23 changes: 16 additions & 7 deletions apps/server/src/core/error/loggable/error.loggable.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,38 @@
import { ValidationError } from '@nestjs/common';
import { ApiValidationError } from '@shared/common';
import { getMetadataStorage } from 'class-validator';
import util from 'util';
import { Loggable } from '../../logger/interfaces';
import { ErrorLogMessage, LogMessageDataObject, ValidationErrorLogMessage } from '../../logger/types';
import { ErrorUtils } from '../utils/error.utils';

export class ErrorLoggable implements Loggable {
constructor(private readonly error: Error, private readonly data?: LogMessageDataObject) {}
readonly actualError: Error;

constructor(private readonly error: unknown, private readonly data?: LogMessageDataObject) {
if (this.error instanceof Error) {
this.actualError = <Error>error;
} else {
this.actualError = new Error(util.inspect(error));
}
}

private readonly classValidatorMetadataStorage = getMetadataStorage();

getLogMessage(): ErrorLogMessage | ValidationErrorLogMessage {
let logMessage: ErrorLogMessage | ValidationErrorLogMessage = {
error: this.error,
error: this.actualError,
type: '',
data: this.data,
};

if (this.error instanceof ApiValidationError) {
logMessage = this.createLogMessageForValidationErrors(this.error);
} else if (ErrorUtils.isFeathersError(this.error)) {
if (this.actualError instanceof ApiValidationError) {
logMessage = this.createLogMessageForValidationErrors(this.actualError);
} else if (ErrorUtils.isFeathersError(this.actualError)) {
logMessage.type = 'Feathers Error';
} else if (ErrorUtils.isBusinessError(this.error)) {
} else if (ErrorUtils.isBusinessError(this.actualError)) {
logMessage.type = 'Business Error';
} else if (ErrorUtils.isNestHttpException(this.error)) {
} else if (ErrorUtils.isNestHttpException(this.actualError)) {
logMessage.type = 'Technical Error';
} else {
logMessage.type = 'Unhandled or Unknown Error';
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/modules/health/health-api.module.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { Module } from '@nestjs/common';

import { LoggerModule } from '@src/core/logger';
import { HealthController } from './controller';
import { HealthCheckRepo } from './repo';
import { HealthService } from './service';
import { HealthUC } from './uc';

@Module({
imports: [LoggerModule],
controllers: [HealthController],
providers: [HealthCheckRepo, HealthService, HealthUC],
})
Expand Down
5 changes: 5 additions & 0 deletions apps/server/src/modules/health/uc/health.uc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest';

import { IConfig } from '@hpi-schul-cloud/commons/lib/interfaces/IConfig';
import { Configuration } from '@hpi-schul-cloud/commons';
import { Logger } from '@src/core/logger';
import { HealthUC } from './health.uc';
import { HealthService } from '../service';
import { HealthStatuses } from '../domain';
Expand All @@ -23,6 +24,10 @@ describe(HealthUC.name, () => {
provide: HealthService,
useValue: createMock<HealthService>(),
},
{
provide: Logger,
useValue: createMock<Logger>(),
},
],
}).compile();
uc = module.get(HealthUC);
Expand Down
9 changes: 7 additions & 2 deletions apps/server/src/modules/health/uc/health.uc.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Injectable } from '@nestjs/common';

import { Logger } from '@src/core/logger';
import { ErrorLoggable } from '@src/core/error/loggable';
import { HealthService } from '../service';
import { HealthConfig } from '../health.config';
import { HealthStatuses, HealthStatusCheck, HealthStatus } from '../domain';
Expand Down Expand Up @@ -27,7 +29,9 @@ function hasMessage(error: unknown): error is { message: string } {

@Injectable()
export class HealthUC {
constructor(private readonly healthService: HealthService) {}
constructor(private readonly healthService: HealthService, private readonly logger: Logger) {
this.logger.setContext(HealthUC.name);
}

checkSelfHealth(): HealthStatus {
// This health check verifies just the correct module setup and doesn't include
Expand All @@ -43,7 +47,7 @@ export class HealthUC {
async checkOverallHealth(): Promise<HealthStatus> {
// The below check allows for turning off the MongoDB dependency on the health check -
// it shouldn't be typically used, but if this health check will be used e.g. in the k8s
// liveness or readiness probes and, for any reason, there would be a need to stop
// liveliness or readiness probes and, for any reason, there would be a need to stop
// including MongoDB check in the overall API health checks, the HEALTH_CHECKS_EXCLUDE_MONGODB
// config var can be set to 'true' to disable it. This way, as currently only this single
// MongoDB check is included in the overall API health checks, the whole health check will
Expand All @@ -70,6 +74,7 @@ export class HealthUC {

await this.healthService.upsertHealthCheckById(healthCheckID);
} catch (error) {
this.logger.warning(new ErrorLoggable(error));
// If any error occurred in the database operation execution it should be indicated
// as a MongoDB check failure (and thus the whole health check should fail).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@ import { Module } from '@nestjs/common';

import { MongoMemoryDatabaseModule } from '@infra/database';
import { HealthApiModule, HealthEntities } from '@src/modules/health';
import { ConfigModule } from '@nestjs/config';
import { createConfigModuleOptions } from '@src/config';
import { serverConfig } from '@modules/server';

/**
* Internal server module used for testing.
* Use the same modules as the InternalServerModule, but with
* the in-memory MongoDB implementation for testing purposes.
*/
@Module({
imports: [MongoMemoryDatabaseModule.forRoot({ entities: [...HealthEntities] }), HealthApiModule],
imports: [
MongoMemoryDatabaseModule.forRoot({ entities: [...HealthEntities] }),
HealthApiModule,
ConfigModule.forRoot(createConfigModuleOptions(serverConfig)),
],
})
export class InternalServerTestModule {}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { Module } from '@nestjs/common';
import { MikroOrmModule } from '@mikro-orm/nestjs';

import { DB_URL, DB_USERNAME, DB_PASSWORD } from '@src/config';
import { DB_URL, DB_USERNAME, DB_PASSWORD, createConfigModuleOptions } from '@src/config';
import { HealthApiModule, HealthEntities } from '@src/modules/health';
import { ConfigModule } from '@nestjs/config';
import { serverConfig } from '@modules/server';

@Module({
imports: [
Expand All @@ -16,6 +18,7 @@ import { HealthApiModule, HealthEntities } from '@src/modules/health';
// debug: true, // use it only for the local queries debugging
}),
HealthApiModule,
ConfigModule.forRoot(createConfigModuleOptions(serverConfig)),
],
})
export class InternalServerModule {}

0 comments on commit 52a9712

Please sign in to comment.