Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BC-8308 add warning if an error occurs during healtch check #5310

Merged
merged 9 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {}
Loading