dynalite icon indicating copy to clipboard operation
dynalite copied to clipboard

any request error takes down the whole database

Open ojii opened this issue 2 years ago • 2 comments

Describe the issue

We use dynalite in CI. An annoying number of times, the database server (which is used by several tests) crashes with ECONNRESET, making the rest of the tests fail.

I believe this is because of the throw in this line.

Could that throw err be changed to something like console.log(err)? I don't understand nodejs http servers enough to know if that is an appropriate solution.

Steps to reproduce

Sadly, I don't have a concise repro for this at the moment.

Expected behavior

A client resetting a connection should not take down the whole server.

ojii avatar Nov 29 '23 07:11 ojii

That seems reasonable to me, although I'd def love to get feedback from other Dynalite users on that change.

ryanblock avatar Nov 29 '23 14:11 ryanblock

So, I've been test running a patched version with the console.error(err) instead of the throw err in there for a while now on our CI and the results are:

  • The ECONNRESET error no longer takes down the whole db and subsequent tests are fine
  • It doesn't fully solve the problem we have where dynalite just becomes unresponsive for a while. This is most likely not a dynalite issue but some problem with our CI setup/test suite.

I still think changing that line is a worthwhile thing to do, but sadly it wasn't the magical silver bullet we were hoping for :D

ojii avatar Dec 05 '23 08:12 ojii