enableExpressErrorHandler option does not work

[hybeats]

Issue Description

enableExpressErrorHandler option does not work. After setting it to 'true', cloud code errors still return { code: '141', error: 'message' }

Steps to reproduce

1. Set enableExpressErrorHandler option to 'true'
   const api = new ParseServer({ databaseURI: databaseUri || 'postgres://postgres:postgres@localhost:5432/postgres', cloud: __dirname + '/cloud/main.js', appId: process.env.APP_ID, masterKey: process.env.MASTER_KEY, restAPIKey: process.env.REST_API_KEY, serverURL: process.env.SERVER_URL, liveQuery: { classNames: ["Posts", "Comments"] }, enableExpressErrorHandler: true, });
2. Write your own error handler middleware
   `const errorHandler = (err, req, res, next) => {
   res.json({ test: '123', err, });
   }

module.exports = errorHandler;
`

3. Use it as last middleware
   `const app = express();

// Serve static assets from the /public folder
app.use('/public', express.static(path.join(__dirname, '/public')));

// Dashboard
app.use('/dashboard', dashboard);

// Serve the Parse API on the /parse URL prefix
const mountPath = '/parse';
app.use(mountPath, api);

// error handler middleware
app.use(errorHandler);`

4. Call non existing could function
   

Expected Results

We get response
{ "test": "123", "err": { "code": 141, "error": "Invalid function: \"qwerty\"" } }

Actual Outcome

Actual response
{ "code": 141, "error": "Invalid function: \"qwerty\"" }

Environment Setup

• Server
  • parse-server version (Be specific! Don't say 'latest'.) : 3.10.0
  • Operating System: Windows 10
  • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): localhost, express

Logs/Trace

error: Error handling request: { Error: Invalid function: "qwerty"
at handleCloudFunction (C:\projects\crecom_backend\node_modules\parse-server\lib\Routers\FunctionsRouter.js:134:13)
at C:\projects\crecom_backend\node_modules\parse-server\lib\PromiseRouter.js:175:7
at Layer.handle [as handle_request] (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\layer.js:95:5)
at next (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\route.js:137:13)
at Route.dispatch (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\route.js:112:3)
at Layer.handle [as handle_request] (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\layer.js:95:5)
at C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:281:22
at param (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:354:14)
at param (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:365:14)
at Function.process_params (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:410:3) message: 'Invalid function: "qwerty"', code: 141 } {"error":{"message":"Invalid function: "qwerty"","code":141}}
error: Parse error: Invalid function: "qwerty" {"code":141,"stack":"Error: Invalid function: "qwerty"\n at handleCloudFunction (C:\projects\crecom_backend\node_modules\parse-server\lib\Routers\FunctionsRouter.js:134:13)\n at C:\projects\crecom_backend\node_modules\parse-server\lib\PromiseRouter.js:175:7\n at Layer.handle [as handle_request] (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\layer.js:95:5)\n at next (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\route.js:137:13)\n at Route.dispatch (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\route.js:112:3)\n at Layer.handle [as handle_request] (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\layer.js:95:5)\n at C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:281:22\n at param (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:354:14)\n at param (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:365:14)\n at Function.process_params (C:\projects\crecom_backend\node_modules\parse-server\node_modules\express\lib\router\index.js:410:3)"}
Error: Can't set headers after they are sent.
at validateHeader (_http_outgoing.js:491:11)
at ServerResponse.setHeader (_http_outgoing.js:498:3)
at ServerResponse.header (C:\projects\crecom_backend\node_modules\express\lib\response.js:695:10)
at ServerResponse.send (C:\projects\crecom_backend\node_modules\express\lib\response.js:146:12)
at ServerResponse.json (C:\projects\crecom_backend\node_modules\express\lib\response.js:235:15)
at errorHandler (C:\projects\crecom_backend\middlewares\errorHandler.js:2:7)
at Layer.handle_error (C:\projects\crecom_backend\node_modules\express\lib\router\layer.js:58:5)
at trim_prefix (C:\projects\crecom_backend\node_modules\express\lib\router\index.js:300:13)
at C:\projects\crecom_backend\node_modules\express\lib\router\index.js:270:7
at Function.proto.process_params (C:\projects\crecom_backend\node_modules\express\lib\router\index.js:321:12)

Additional info

I looked into your middleware code and noticed, that your errorHandlerMiddleware:
`export function handleParseErrors(err, req, res, next) {
const log = (req.config && req.config.loggerController) || defaultLogger;
if (err instanceof Parse.Error) {
let httpStatus;
// TODO: fill out this mapping
switch (err.code) {
case Parse.Error.INTERNAL_SERVER_ERROR:
httpStatus = 500;
break;
case Parse.Error.OBJECT_NOT_FOUND:
httpStatus = 404;
break;
default:
httpStatus = 400;
}

res.status(httpStatus);
res.json({ code: err.code, error: err.message });
log.error('Parse error: ', err);
if (req.config && req.config.enableExpressErrorHandler) {
  next(err);
}

}`

It executes res.json first, and next(err) after. So we cant actually control output anymore, since headers are already sent

I would place this condition at the beginning and executed return next(err), like that, so we can control output after:

`export function handleParseErrors(err, req, res, next) {
const log = (req.config && req.config.loggerController) || defaultLogger;
if (err instanceof Parse.Error) {

if (req.config && req.config.enableExpressErrorHandler) {
  return next(err);
}

let httpStatus;
// TODO: fill out this mapping
switch (err.code) {
  case Parse.Error.INTERNAL_SERVER_ERROR:
    httpStatus = 500;
    break;
  case Parse.Error.OBJECT_NOT_FOUND:
    httpStatus = 404;
    break;
  default:
    httpStatus = 400;
}
res.status(httpStatus);
res.json({ code: err.code, error: err.message });
log.error('Parse error: ', err);

}`

I tried this out, and got this response, as expected:

[davimacedo]

Hi @hybeats . Thanks for detailed reporting. Would you be willed to send a PR with your fix suggestion and a test case?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.