Skip to content

Prevent crash when req.logger is undefined in error handler #2765

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.

Sign up for GitHub

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

Open
zackcl wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Prevent crash when req.logger is undefined in error handler #2765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1380fa
Prevent crash when req.logger is undefined in ErrorHandlerMiddleware
zackcl Dec 1, 2025
9dcd968
Remove async/await from tests and add undefined logger test
zackcl Dec 1, 2025
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
11 changes: 4 additions & 7 deletions backend/packages/Upgrade/src/api/middlewares/ErrorHandlerMiddleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ export class ErrorHandlerMiddleware implements ExpressErrorMiddlewareInterface {

constructor(public errorService: ErrorService) {}

public async error(
error: any,
req: express.Request,
res: express.Response,
next: express.NextFunction
): Promise<void> {
public error(error: any, req: express.Request, res: express.Response, next: express.NextFunction): void {
// It seems like some decorators handle setting the response (i.e. class-validators)
let message: string;
let type: SERVER_ERROR;
Expand Down Expand Up @@ -123,7 +118,9 @@ export class ErrorHandlerMiddleware implements ExpressErrorMiddlewareInterface {
// #1042 send request in logging output, don't need to put into database
experimentError.request = req.body;

req.logger.error(experimentError);
if (req.logger) {
req.logger.error(experimentError);
}

// #1040
// experimentError.type ? await this.errorService.create(experimentError, req.logger) : await Promise.resolve(error);
Expand Down
83 changes: 53 additions & 30 deletions backend/packages/Upgrade/test/unit/middlewares/ErrorHandlerMiddleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,174 +36,197 @@ describe('ErrorHandler Middleware tests', () => {
};
});

test('Incorrect param error test', async () => {
test('Incorrect param error test', () => {
const error = {
type: SERVER_ERROR.INCORRECT_PARAM_FORMAT,
message: 'Incorrect Parameters',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Missing param error test', async () => {
test('Missing param error test', () => {
const error = {
type: SERVER_ERROR.MISSING_PARAMS,
message: 'Missing Parameters',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Query failed error test', async () => {
test('Query failed error test', () => {
const error = {
type: SERVER_ERROR.QUERY_FAILED,
message: 'Query failed',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Experiment user not defined error test', async () => {
test('Experiment user not defined error test', () => {
const error = {
type: SERVER_ERROR.EXPERIMENT_USER_NOT_DEFINED,
message: 'Experiment user not defined',
httpCode: 404,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Experiment user group not defined error test', async () => {
test('Experiment user group not defined error test', () => {
const error = {
type: SERVER_ERROR.EXPERIMENT_USER_GROUP_NOT_DEFINED,
message: 'Experiment user group not defined',
httpCode: 404,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Assignment error test', async () => {
test('Assignment error test', () => {
const error = {
type: SERVER_ERROR.ASSIGNMENT_ERROR,
message: 'Assignment error',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Working group not subset of group error test', async () => {
test('Working group not subset of group error test', () => {
const error = {
type: SERVER_ERROR.WORKING_GROUP_NOT_SUBSET_OF_GROUP,
message: 'Working group not subset of group error',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Invalid token error test', async () => {
test('Invalid token error test', () => {
const error = {
type: SERVER_ERROR.INVALID_TOKEN,
message: 'Invalid token error',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Token not present error test', async () => {
test('Token not present error test', () => {
const error = {
type: SERVER_ERROR.TOKEN_NOT_PRESENT,
message: 'Token not present error',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Condition not found error test', async () => {
test('Condition not found error test', () => {
const error = {
type: SERVER_ERROR.CONDITION_NOT_FOUND,
message: 'Condition not found error',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Email send error test', async () => {
test('Email send error test', () => {
const error = {
type: SERVER_ERROR.EMAIL_SEND_ERROR,
message: 'Email send error',
httpCode: 500,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Incorrect param format error test', async () => {
test('Incorrect param format error test', () => {
const error = {
message: 'Incorrect param format error',
httpCode: 400,
};
await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('User not found error test', async () => {
test('User not found error test', () => {
const error = {
message: 'User not found errors',
httpCode: 401,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Query failed error test', async () => {
test('Query failed error test', () => {
const error = {
message: 'Query failed error',
httpCode: 404,
};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});

test('Default error test', async () => {
test('Default error test', () => {
const error = {};

await errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
errorhandler.error(error, mockRequest, mockResponse as Response, nextFunction);
expect(mockResponse.statusCode).toBe(500);
});

test('Error handler should not crash when logger is undefined', () => {
const mockRequestWithoutLogger: any = {
header: jest.fn(),
body: { test: 'data' },
originalUrl: '/test/endpoint',
};

const error = {
type: SERVER_ERROR.QUERY_FAILED,
message: 'Query failed',
httpCode: 500,
name: 'TestError',
};

// This should not throw an error even though logger is undefined
expect(() => {
errorhandler.error(error, mockRequestWithoutLogger, mockResponse as Response, nextFunction);
}).not.toThrow();

expect(mockResponse.statusCode).toBe(error.httpCode);
expect(mockResponse.body).toBe(error.message);
});
});