Responsibly aborting in node

[Raynos]

Based on a conversation with @benjamingr ( groundwater/nodejs-symposiums#5 (comment) ).

We recommend aborting and core dumps as the debugging best practice. However we don't document how to do so responsibly to ensure uptime and availability.

People object to exiting and restarting as its a potential DDOS attack vector, I've brushed this off in the past by saying "it's a bug. Just get paged and fix this bug", however I no longer think this can be ignored for a few reasons

• I've seen accidental internal DDOS due to cascading failures.
• I've seen how even 10 minutes of outage can lose millions in revenue.
• I've even seen accidental DDOS simply due to an embedded gossip library having a corruptive bug.
• I've seen a fleet of servers being destroyed due to a single user uuid and round robin load balancing

I 100% believe a core dump is the only way to debug that 1 in a 100 edgecase. We all speak positively about how amazing post mortem debugging is but there is little sharing around how to do so responsibly.

Here is a straw man suggestion and I would love feedback:

• a programmer error is a bug and needs human attention to verify any undefined state. We recommend setting up alerts or email notifications and verifying the impact of the programmer error.
• running an available service is important, you cannot just abort without consequence. You need a technique to avoid repeated aborting.

We could implement --abort-on-uncaught-exception-once=filepath flag.

• it would check the file on filepath, if it exists, read the value of the file as Unix time stamp (second granularity).
• if the time is less then 24hr in the past then instead of aborting create an error and pass it to process.on(uncaughtException)
• if the file does not exist or time is greater then 24hr in the past then write the current Unix time stamp (second granularity) to the file then continue to abort.

This functionality could be implemented outside of node core. If it is we have to clearly in the node core documentation, instruct people how to use --abort-on-uncaught-exception safely.

Because the process will not abort for 24hr the application developer must use process.on(uncaughtException) to avoid exiting and to be able to degrade to partial availability.

I would love to hear some feedback on how to best responsibly use post mortem debugging techniques

[benjamingr]

The biggest concern I have (from that thread) is with abort on throw.

Aborting on every uncaught exception (like JSON.parse or accessing an unknown property) effectively means anyone who finds the smallest "TypeError: cannot read property foo of undefined" the code base can single-handedly cause a denial of service attack on the servers by sending a low amount of requests to it.

I'd like to share that I have similar experience to @Raynos - both in my code base and when I did contract work for a Node project of a large company. I've seen "throw on abort" cause denial of service attacks in the real world .

---

I think we can also agree that letting exceptions "escape" and running process.on("uncaughtException", () => ...) leaves the server at indeterminate state which can be very bad and cause it - for example - to hold connections to the database and cascade the failure on.

Note that unlike that, promise rejections play nice with the call-stack and will run cleanup code at every phase of the chain meaning that a promise rejection doesn't lead to undefined state in the server. It can be handled at the process level without leaking resources - using patterns similar to Go's defer keyword and C++'s RAII.

However, promise rejections currently have a problematic post mortem story for people who debug with heap-dumps because the error is rejected after cleanup has been performed and the stack has been unwound. So while you get a nice stack trace and avoid indeterminate state - you don't get a nice heap dump when the error occurred. This problem can possibly be amplified when async/await lands and more people use promises.

Note that it's not specific to promises, basically what we need isn't promises per-se it's async pieces of code that run a finally logic for resource clean up so the code is always in determined clean state. Promises just happen to be able to do that but it's not a capability that's tied to promises, core could just as easily introduce a mechanism without them.

I'd like to see how we can both have cleanup and a nice heap dump. Tricks like "the first rejection causes a heap dump and the the following contain regular behavior can be considered.

--abort-on-uncaught-exception-once is a good idea but it sounds problematic because it leaves the code in undetermined state, if we had a cleanup mechanism for when a callback finished or something similar. --abort-on-first-rejection-once might be interesting if the heap dump can be fixed which will mean we get the best of both worlds.

[bnoordhuis]

running an available service is important, you cannot just abort without consequence. You need a technique to avoid repeated aborting.

Why is that? To avoid the disk from filling up? I'd say that's the job of abrtd or systemd or whatever you use to collect and manage core dumps.

If it's because the time it takes to dump core is a service outage, I think you can solve that by calling fork() before abort().

[benjamingr]

Why is that? To avoid the disk from filling up? I'd say that's the job of abrtd or systemd or whatever you use to collect and manage core dumps.

Because the time to restart a node server is typically a few seconds. Let's say it's 8 for a specific instance and I have two servers with 4 cores each running two instances of node (so 8 server workers total). This means that I have to send the server 1 request per second to that particular path and I can single handedly take it down.

[Raynos]

@bnoordhuis

Disk filling up does not matter. What matters is downtime.

I do not believe you can fork before abort as you would be forking the undefined behaviour process into a new undefined behaviour process.

The issue is that process startup is generally not cheap. At least 2s ranging up to 5s.

If your service does 2000 qps per process and any request can cause a crash then you just lost 4k requests. If you have retries (you should) then the retry storm can bring your entire deployment to its knees.

Now imagine that your not running a stateless service. You might have to load data at startup. You might have to gossip. You might have to replicate. You might have to migrate. You might have to do leadership election.

I've seen bad cascading failures and they generally happen at >100 uncaughts per second.

Even if you have a fleet of workers available you need to be able to pre-spawn workers. Note this is a valid solution; we should document any solution that achieves good uptime under an "uncaught storm". The kind of bug with every request. This is generally a bug created by new code and fixed by rollback. However rollback takes time and there is no reason to be down for 5 minutes.

[bnoordhuis]

I do not believe you can fork before abort as you would be forking the undefined behaviour process into a new undefined behaviour process.

Why would that matter if you're going to call abort() immediately afterwards anyway? To clarify, I had the impression you wanted to truck on after an error (possibly using process.on('uncaughtException') and may $DEITY have mercy on your soul in that case) but still get a core dump; fork + abort (in C++) will accomplish that.

[Raynos]

@bnoordhuis

Can I disable --abort-on-uncaught-exception after forking ?

That would actually have the correct semantics and should in theory take less time then a restart. In fact if we could have the exact same error go to process.on(uncaughtException) in the forked process that would be beautiful.

I rather like this idea as its a truly abort once approach.

[Raynos]

Note to self: mobile GitHub puts the close button in a bad place.

[bnoordhuis]

Can I disable --abort-on-uncaught-exception after forking ?

I don't see why not. Just call isolate->SetAbortOnUncaughtExceptionCallback(nullptr) afterwards in the original process.

[Raynos]

@bnoordhuis

Sounds like a good technical solution.

Do you think this is something worth recommending ? Should we recommend to never continue on uncaughts ? What is a responsible default approach to recommend for production ?

Alternatively we can also recommend "here are some possible options; figure it out yourself!"

[bnoordhuis]

Should we recommend to never continue on uncaughts ?

I put those big scary warnings in the documentation for a reason. :-)

Alternatively we can also recommend "here are some possible options; figure it out yourself!"

That's what I would do. If the pros and cons are clearly spelled out, people can make their own informed decision.

[davepacheco]

@Raynos @bnoordhuis I agree with the idea of spelling out various options, pros, and cons, and maybe even providing concrete references if companies are willing to do that. I think there are strongly divergent opinions on these things and many of these come down to policy or value choices that may legitimately differ across teams.