20

OK, so I have a problem. If an uncaught exception occurs while I am handling an HTTP request, I have no opportunity to call the end() method on the http.ServerResponse object. Therefore, the server hangs forever and never fulfills the request.

Here's an example:

var express = require('express');
var app = express.createServer();
var reqNum = 0;
app.get('/favicon.ico', function(req, res) {res.send(404);});
app.get('*', function(req, res, next) {
    console.log("Request #", ++reqNum, ":", req.url);
    next();
});
app.get('/error', function(req, res, next) {
    throw new Error("Problem occurred");
});
app.get('/hang', function(req, res, next) {
    console.log("In /hang route");
    setTimeout(function() {
        console.log("In /hang callback");
        if(reqNum >= 3)
            throw new Error("Problem occurred");
        res.send("It worked!");
    }, 2000);
});
process.on('uncaughtException', function(err) {
    console.log("Uncaught exception!", err);
});
app.listen(8080);

If you visit /error, an exception occurs, but it is caught. The user receives an error message - no problem. If I visit /hang, though, the server will eventually throw an uncaught exception and hang forever. Any subsequent requests for /hang will hang.

This sucks. Any advice for how to fix this issue?

BMiner
  • 16,669
  • 12
  • 53
  • 53

3 Answers3

16

When an uncaught exception occurs, you're in an unclean state. Let the process die and restart it, there's nothing else you can do to safely bring it back to a known-good state. Use forever, it'll restart your process as soon as it dies.

Dr McKay
  • 2,548
  • 2
  • 21
  • 26
thejh
  • 44,854
  • 16
  • 96
  • 107
  • 9
    I agree to a certain extent. In production environments, you can't really shutdown the whole server just because of a single uncaught exception. What if a single PHP page had an issue? Would your whole web server go down? I was going to have the server email me if an uncaught exception occurs and automatically shutdown if more than 10 occur within a 60 second period. But, really... I don't want the server to go down and reboot. I'd like to try and recover. – BMiner Nov 13 '11 at 22:04
  • @BMiner: You can use hook.io or so to split your app in multiple processes and the builtin cluster stuff to make seperate worker processes - this way, not everything goes down, just a part needs to be restarted. – thejh Nov 13 '11 at 23:00
  • 4
    @BMiner Restarting your application is easier than apologizing client for corruption of they data. As thejh said, runtime is in unclean state. From now on, you should assume that everything is an undefined behavior. – Dr McKay Nov 13 '11 at 23:05
  • 1
    @thejh Why must I assume that I am in an unclean state? Maybe only a part of the program is broken? Why is it safer to just shutdown and restart? I believe you guys, but I am still skeptical. Could you provide an example? – BMiner Nov 13 '11 at 23:29
  • 4
    @BMiner: You're saying "maybe". The thing is, a `throw` is like a GOTO, it jumps out of your code when stuff is half done, and now your state might be corrupt. Example: `user.name = newname; db.save(user.id, 'name', newname)` - what if `db.save` breaks before it completes? Your DB contains a different name than the one you have in RAM. – thejh Nov 13 '11 at 23:46
  • 3
    Completely agree with @thejh you should let the process die immediately (to avoid data coruption etc) and then restart your server. – alessioalex Nov 14 '11 at 09:11
  • @thejh Fair enough. Thanks for your feedback and help. – BMiner Nov 15 '11 at 21:22
  • 1
    I totally agree that the process should die -- but would it be so wrong to "try" to send a 500 response of some sort to the end user.? – Skone Jul 25 '14 at 18:45
  • 1
    My gut feeling tells me that this is not right: 1. what about server reliability, what about other queries currently pending in the server? 2. if there is state inconsistency, there are two possibilities: 2.1. there is state kept in function call stack, which will be cleared by throw; 2.2. there is state kept in global variables, which is considered as a bad practise, and if server restarted, won't it cause even more problem because it gets re-initialized? Throw is not GOTO and it is not a problem, catch it at right place is the key. – xiGUAwanOU Jan 22 '19 at 15:36
1

If error is thrown synchronously, express won't stop working, only returning 500.

this.app.get("/error", (request, response) => {
  throw new Error("shouldn't stop");
});

If error is thrown asynchronously, express will crash. But according to it's official documentation, there is still a way to recover from it by calling next:

this.app.get("/error", (request, response, next) => {
  setTimeout(() => {
    try {
      throw new Error("shouldn't stop");
    } catch (err) {
      next(err);
    }
  }, 0);
});

This will let express do its duty to response with a 500 error.

xiGUAwanOU
  • 325
  • 2
  • 16
-1

Use try/catch/finally.

app.get('/hang', function(req, res, next) {
    console.log("In /hang route");
    setTimeout(function() {
        console.log("In /hang callback");
        try {
            if(reqNum >= 3)
                throw new Error("Problem occurred");
        } catch (err) {
            console.log("There was an error", err);
        } finally {
            res.send("It worked!");
        }
    }, 2000);
});
fent
  • 17,861
  • 15
  • 87
  • 91