0

I'm running a simple Fastify server and when I make a request and it fails, I wanna handle that exception in the setErrorHandler.

How can I achieve that?

This doesn't seem to work:

import fastify from 'fastify';
import fetch from 'node-fetch';

const app = fastify();

app.setErrorHandler(async (error, request, response) => {
    // I want this to be called whenever there is an error
    console.log('setErrorHandler');
    return 'error';
});

app.get('/', (request, response) => {
    // I do not want to use async / await or .catch here
    fetch('https://...').then(() =>  { response.send(); });
});

I get the error UnhandledPromiseRejectionWarning and the server never sends a response.

I do NOT want to use async, await or .catch on the promise. The reason is I'm simulating a developer error. So if someone forgets to add a .catch and is not using async / await, I still wanna "catch" that error and return status 500 to the client.

I'm willing to change / wrap the request library adding a .catch in it, I just don't wanna change that endpoint handler code in general, since it's sort of out of my control (another developer might code it any way they want).

Rodrigo Ruiz
  • 4,248
  • 6
  • 43
  • 75
  • Help me to understand why not follow the guidelines stated in the Fastify documentation: https://www.fastify.io/docs/master/Errors/#catching-errors-in-promises _"While unhandledRejection is deprecated in Node.js, unhandled rejections will not throw, and still potentially leak. You should use a module like make-promises-safe to ensure unhandled rejections always throw."_. make-promises-safe lib: https://github.com/mcollina/make-promises-safe – Manny Aug 10 '21 at 00:49
  • Reading the documentation, I am not sure one can set the `setErrorHandler` to catch `UnhandledRejection`s – Manny Aug 10 '21 at 00:54
  • :/ Looking at `make-promises-safe` it looks to make use of the `process.on('unhandledRejection')` event listener and exists out with code 1. The suggestion I was going to make prior to what I wrote above was to make use of this event listener and somehow return from that point... – Manny Aug 10 '21 at 00:57
  • `// I do not want to use async / await or .catch here` - Well, `.catch()` is how you catch promise rejections that you aren't using `await` on. If you're programming with promises, you HAVE to write proper error handling. It's just shirking your responsibilities as a good programmer to not do so. I'd suggest you stop looking for a way to not write proper error handling. – jfriend00 Aug 10 '21 at 01:44
  • @Manny the `make-promise-safe` module would not return any response to the client (I tested it). As for the why doing it that way, I wanna protect the client from developer error. If one of the developers makes a mistake and forgets to add the ".catch", how would I still send a 500 response to the client? – Rodrigo Ruiz Aug 10 '21 at 04:09
  • @jfriend00 it won't, I also tested it. It only works if I use `await` on the request promise. If the developer doesn't use await (for whatever reason), even with the `async` on the upper level callback, it won't bubble up the exception. – Rodrigo Ruiz Aug 10 '21 at 04:11
  • I deleted my answer because this is just kind of a silly question. Handle errors INSIDE the route handler or make sure they propagate back to the Fastify error handler. That's the ONLY safe thing to do. Shirking that responsibility only leads to bad code and a potentially unreliable server. I'm regret even trying to offer suggestions. The only good advice is that whoever writes a route handler, NEEDS to handle their own errors. Stop trying to protect bad developers. Make them do proper error handling. – jfriend00 Aug 10 '21 at 04:28
  • @jfriend00 How do I propagate the error to the error handler if it happens inside a promise (without using `await`)? – Rodrigo Ruiz Aug 10 '21 at 04:53
  • @RodrigoRuiz - You either use `await` or you return the promise from the route handler. There is no other way to do it. That's how you propagate rejections back to the caller. This is how promises are designed. The only other place to intercept anything is the global `uncaughtException` event, but that doesn't give you the context you need to send an error response and is generally a bad design idea anyway. Errors should be handled locally. – jfriend00 Aug 10 '21 at 05:02

1 Answers1

3

The reason is I'm simulating a developer error.

You can't manage these errors at runtime to reply to a request because you don't reference the reply object to act accordingly.

You can catch those errors with, but the reply cannot be fulfilled:

process.on('unhandledRejection', (err) => {
  console.log(err)
})

As an alternative, I would set connectionTimeout, so the client will get a timeout error at least.

As you wrote, you already know that you should change the handler code to let Fastify be aware of the promise:

// add return
app.get('/', (request, response) => {
    return fetch('https://...').then(() =>  { response.send(); });
})

For this reason, I think the solution to your problem should be taken offline adding a linter rule (like so) and integrate some Static Code Analysis in the CI pipeline to reject bad code.

Manuel Spigolon
  • 11,003
  • 5
  • 50
  • 73
  • Yeah... I'm getting to the conclusion that there isn't a good solution to what I'm looking for in JS. The closest I got was by using `await` on every promise, allowing Fastify to catch it in the error handler. Also I could return an `UnexpectedError` from whatever promise needs it and handle it in the logic. It's just that it doesn't seem like an elegant solution to me. – Rodrigo Ruiz Aug 10 '21 at 17:18
  • May I suggest asking this in https://github.com/fastify/help, the maintenance established that so that the community may directly interact and get their questions answered. Maybe Matteo and team can provide additional insight and guidance. – Manny Aug 10 '21 at 22:36