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

[TTAHUB-3097] Implement transactions for worker processes #2233

Merged
merged 25 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion email_templates/changes_requested_by_manager/html.pug
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ style
include ../email.css
p Hello,
p
p #{managerName} requested changed to report #{displayId}.
p #{managerName} requested changes to report #{displayId}.
GarrettEHill marked this conversation as resolved.
Show resolved Hide resolved
if comments
p #{managerName} provided the following comments:
blockquote !{comments}
Expand Down
179 changes: 133 additions & 46 deletions src/lib/apiErrorHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,28 @@ import { sequelize } from '../models';
* @returns {Promise<number|null>} - The ID of the stored request error, or null if storing failed.
*/
async function logRequestError(req, operation, error, logContext) {
// Check if error logging should be suppressed
if (
operation !== 'SequelizeError'
&& process.env.SUPPRESS_ERROR_LOGGING
&& process.env.SUPPRESS_ERROR_LOGGING.toLowerCase() === 'true'
) {
return 0;
}
if (!error) {
return 0;
}

try {
// Prepare the response body for storage
const responseBody = typeof error === 'object'
&& error !== null ? { ...error, errorStack: error?.stack } : error;
const responseBody = typeof error === 'object' && error !== null
GarrettEHill marked this conversation as resolved.
Show resolved Hide resolved
? { ...error, errorStack: error?.stack }
: error;

// Prepare the request body for storage
const requestBody = {
...(req.body
&& typeof req.body === 'object'
&& Object.keys(req.body).length > 0
&& { body: req.body }),
...(req.params
&& typeof req.params === 'object'
&& Object.keys(req.params).length > 0
&& { params: req.params }),
...(req.query
&& typeof req.query === 'object'
&& Object.keys(req.query).length > 0
&& { query: req.query }),
...(req.body && typeof req.body === 'object' && Object.keys(req.body).length > 0 && { body: req.body }),
...(req.params && typeof req.params === 'object' && Object.keys(req.params).length > 0 && { params: req.params }),
...(req.query && typeof req.query === 'object' && Object.keys(req.query).length > 0 && { query: req.query }),
};

// Create a request error in the database and get its ID
GarrettEHill marked this conversation as resolved.
Show resolved Hide resolved
const requestErrorId = await createRequestError({
operation,
uri: req.originalUrl,
Expand All @@ -69,15 +60,13 @@ async function logRequestError(req, operation, error, logContext) {
* @param {Object} logContext - The context for logging.
*/
export const handleError = async (req, res, error, logContext) => {
// Check if the environment is development
if (process.env.NODE_ENV === 'development') {
logger.error(error);
}

let operation;
let label;

// Check if the error is an instance of Sequelize.Error
if (error instanceof Sequelize.Error) {
operation = 'SequelizeError';
label = 'Sequelize error';
Expand All @@ -86,27 +75,17 @@ export const handleError = async (req, res, error, logContext) => {
label = 'UNEXPECTED ERROR';
}

// eslint-disable-next-line max-len
if (error instanceof Sequelize.ConnectionError || error instanceof Sequelize.ConnectionAcquireTimeoutError) {
if (error instanceof Sequelize.ConnectionError
|| error instanceof Sequelize.ConnectionAcquireTimeoutError) {
const pool = sequelize?.connectionManager?.pool;
const usedConnections = pool ? pool?.used?.length : null;
const waitingConnections = pool ? pool?.pending?.length : null;
const usedConnections = pool ? pool.used.length : null;
const waitingConnections = pool ? pool.pending.length : null;
logger.error(`${logContext.namespace} Connection Pool: Used Connections - ${usedConnections}, Waiting Connections - ${waitingConnections}`);
}

// Log the request error and get the error ID
const requestErrorId = await logRequestError(req, operation, error, logContext);

let errorMessage;
const errorMessage = error?.stack || error;

// Check if the error has a stack property
if (error?.stack) {
errorMessage = error.stack;
} else {
errorMessage = error;
}

// Log the error message with the error ID if available
if (requestErrorId) {
logger.error(`${logContext.namespace} - id: ${requestErrorId} ${label} - ${errorMessage}`);
} else {
Expand All @@ -117,12 +96,11 @@ export const handleError = async (req, res, error, logContext) => {
};

/**
* Handles any unexpected errors in an error handler catch block
*
* @param {*} req - request
* @param {*} res - response
* @param {*} error - error
* @param {*} logContext - useful data for logging
* Handles any unexpected errors in an error handler catch block.
* @param {Object} req - The request object.
* @param {Object} res - The response object.
* @param {Error} error - The error object.
* @param {Object} logContext - The context for logging.
*/
export function handleUnexpectedErrorInCatchBlock(req, res, error, logContext) {
logger.error(`${logContext.namespace} - Unexpected error in catch block - ${error}`);
Expand All @@ -131,11 +109,10 @@ export function handleUnexpectedErrorInCatchBlock(req, res, error, logContext) {

/**
* Handles API errors. Saves data in the RequestErrors table and sends 500 error.
*
* @param {*} req - request
* @param {*} res - response
* @param {*} error - error
* @param {*} logContext - useful data for logging
* @param {Object} req - The request object.
* @param {Object} res - The response object.
* @param {Error} error - The error object.
* @param {Object} logContext - The context for logging.
*/
export default async function handleErrors(req, res, error, logContext) {
try {
Expand All @@ -144,3 +121,113 @@ export default async function handleErrors(req, res, error, logContext) {
handleUnexpectedErrorInCatchBlock(req, res, e, logContext);
}
}

/**
* Logs a worker error and stores it in the database.
* @param {Object} job - The job object.
* @param {string} operation - The operation name.
* @param {Error} error - The error object.
* @param {Object} logContext - The logging context.
* @returns {Promise<number|null>} - The ID of the stored request error, or null if storing failed.
*/
const logWorkerError = async (job, operation, error, logContext) => {
if (
operation !== 'SequelizeError'
&& process.env.SUPPRESS_ERROR_LOGGING
&& process.env.SUPPRESS_ERROR_LOGGING.toLowerCase() === 'true'
) {
return 0;
}

try {
const responseBody = typeof error === 'object' && error !== null
? { ...error, errorStack: error?.stack }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't ...error also contain error.stack. Why do we need to put it in the property errorStack?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just matching what the original code had

: error;

const requestBody = {
...(job.data && typeof job.data === 'object' && Object.keys(job.data).length > 0 && { data: job.data }),
};

const requestErrorId = await createRequestError({
operation,
uri: job.queue.name,
method: 'PROCESS_JOB',
requestBody,
responseBody,
responseCode: INTERNAL_SERVER_ERROR,
});

return requestErrorId;
} catch (e) {
logger.error(`${logContext.namespace} - Sequelize error - unable to store RequestError - ${e}`);
}

return null;
};

/**
* Handles errors in a worker job.
* @param {Object} job - The job object.
* @param {Error} error - The error object.
* @param {Object} logContext - The context for logging.
*/
export const handleWorkerError = async (job, error, logContext) => {
if (process.env.NODE_ENV === 'development') {
logger.error(error);
}

let operation;
let label;

if (error instanceof Sequelize.Error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we check instanceof Sequelize.Error here but above we do operation !== 'SequelizeError'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just matching original logic

operation = 'SequelizeError';
label = 'Sequelize error';
} else {
operation = 'UNEXPECTED_ERROR';
label = 'UNEXPECTED ERROR';
}

if (error instanceof Sequelize.ConnectionError
|| error instanceof Sequelize.ConnectionAcquireTimeoutError) {
const pool = sequelize?.connectionManager?.pool;
const usedConnections = pool ? pool.used.length : null;
const waitingConnections = pool ? pool.pending.length : null;
logger.error(`${logContext.namespace} Connection Pool: Used Connections - ${usedConnections}, Waiting Connections - ${waitingConnections}`);
}

const requestErrorId = await logWorkerError(job, operation, error, logContext);

const errorMessage = error?.stack || error;

if (requestErrorId) {
logger.error(`${logContext.namespace} - id: ${requestErrorId} ${label} - ${errorMessage}`);
} else {
logger.error(`${logContext.namespace} - ${label} - ${errorMessage}`);
}

// Handle job failure as needed
};

/**
* Handles any unexpected errors in a worker error handler catch block.
* @param {Object} job - The job object.
* @param {Error} error - The error object.
* @param {Object} logContext - The context for logging.
*/
export const handleUnexpectedWorkerError = (job, error, logContext) => {
logger.error(`${logContext.namespace} - Unexpected error in catch block - ${error}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will call stack be visible here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think so

};

/**
* Handles worker job errors. Logs the error and stores it in the database.
* @param {Object} job - The job object.
* @param {Error} error - The error object.
* @param {Object} logContext - The context for logging.
*/
export const handleWorkerErrors = async (job, error, logContext) => {
try {
await handleWorkerError(job, error, logContext);
} catch (e) {
handleUnexpectedWorkerError(job, e, logContext);
}
};
Loading