Repository: directus/directus
File: api/src/middleware/error-handler.ts (TypeScript)

Code before:
import { ErrorCode, MethodNotAllowedError, isDirectusError } from '@directus/errors';
import { isObject, toArray } from '@directus/utils';
import { getNodeEnv } from '@directus/utils/node';
import type { ErrorRequestHandler } from 'express';
import getDatabase from '../database/index.js';
import emitter from '../emitter.js';
import { useLogger } from '../logger.js';

// Note: keep all 4 parameters here. That's how Express recognizes it's the error handler, even if
// we don't use next
const errorHandler: ErrorRequestHandler = (err, req, res, _next) => {
	const logger = useLogger();

	let payload: any = {
		errors: [],
	};

	const errors = toArray<unknown>(err);

	let status: number | null = null;

	for (const error of errors) {
		if (getNodeEnv() === 'development') {
			if (isObject(error)) {
				error['extensions'] = {
					...(error['extensions'] || {}),
					stack: error['stack'],
				};
			}
		}

		if (isDirectusError(error)) {
			logger.debug(error);

			if (!status) {
				status = error.status;
			} else if (status !== error.status) {
				status = 500;
			}

			payload.errors.push({
				message: error.message,
				extensions: {
					code: error.code,
					...(error.extensions ?? {}),
				},
			});

			if (isDirectusError(error, ErrorCode.MethodNotAllowed)) {
				res.header('Allow', (error as InstanceType<typeof MethodNotAllowedError>).extensions.allowed.join(', '));
			}
		} else {
			logger.error(error);

			status = 500;

			if (req.accountability?.admin === true) {
				const localError = isObject(error) ? error : {};
				const message = localError['message'] ?? typeof error === 'string' ? error : null;

				payload = {
					errors: [
						{
							message: message || 'An unexpected error occurred.',
							extensions: {
								code: 'INTERNAL_SERVER_ERROR',
								...(localError['extensions'] ?? {}),
							},
						},
					],
				};
			} else {
				payload = {
					errors: [
						{
							message: 'An unexpected error occurred.',
							extensions: {
								code: 'INTERNAL_SERVER_ERROR',
							},
						},
					],
				};
			}
		}
	}

	res.status(status ?? 500);

	emitter
		.emitFilter(
			'request.error',
			payload.errors,
			{},
			{
				database: getDatabase(),
				schema: req.schema,
				accountability: req.accountability ?? null,
			},
		)
		.then((updatedErrors) => {
			return res.json({ ...payload, errors: updatedErrors });
		});
};

export default errorHandler;


Review comment (refactor):
I dont necessarily think this abstraction is an improvement over my last review but it also doesnt hurt anyone 🤷 Could we however still add back the original comment that this function requires to have 4 parameters to be marked as global error handler 😄

Code after:
				// Don't expose unknown errors to non-admin users
				errors = [{ message: FALLBACK_ERROR.message, extensions: { code: FALLBACK_ERROR.code } }];
			}
		}
	}

	res.status(status ?? FALLBACK_ERROR.status);

	const updatedErrors = await emitter.emitFilter(
		'request.error',
		errors,
		{},
		{
			database: getDatabase(),
			schema: req.schema,
			accountability: req.accountability ?? null,
		},
	);

	return res.json({ errors: updatedErrors });
});

function asyncErrorHandler(
	fn: (...args: Parameters<ErrorRequestHandler>) => Promise<any>,
): (...args: Parameters<ErrorRequestHandler>) => Promise<ReturnType<ErrorRequestHandler>> {
	return (err, req, res, next) =>
		fn(err, req, res, next).catch((error) => {
			// To be on the safe side and ensure this doesn't lead to an unhandled (potentially crashing) error
			try {
				const logger = useLogger();
				logger.error(error, 'Unexpected error in error handler');
			} catch {
				// Ignore
			}

			// Delegate to default error handler to close the connection
			if (res.headersSent) return next(err);

			res.status(FALLBACK_ERROR.status);
			return res.json({ errors: [{ message: FALLBACK_ERROR.message, extensions: { code: FALLBACK_ERROR.code } }] });
		});
}