Improve error handling for database problems #517
Conversation
andyw8
force-pushed
the
andyw8/improve-error-handling-for-database-problems
branch
from
271cf38
to
44cf6a1
Compare
| @@ -103,6 +104,9 @@ def start | |||
| end | |||
|
|
|||
| def execute(request, params) | |||
| request_name = request | |||
There was a problem hiding this comment.
This was in the wrong place previously.
|
Looking into adding tests for this. |
andyw8
force-pushed
the
andyw8/improve-error-handling-for-database-problems
branch
from
44cf6a1
to
df0c0c1
Compare
|
Added tests. |
andyw8
deleted the
andyw8/improve-error-handling-for-database-problems
branch
|
Thanks for the quick turnaround! Really appreciated 🙇♂️ |
| @@ -19,6 +19,7 @@ def send_message(message) | |||
| # Log a message to the editor's output panel | |||
| def log_message(message) | |||
| $stderr.puts(message) | |||
| send_message({ result: nil }) | |||
There was a problem hiding this comment.
This is indeed the right solution as we need to return something even when errors occur. But this is not the right place to put it.
The goal of the log_message method is to allow both the server and its add-ons to send debug messages to the output tab, even if disconnected from providing a result.
If a database error was to occur in a notification, where no response is expected, then we'd be writing a nil result to the pipe, which wouldn't be read by the notification. The next request would then incorrectly read it and we'd end up with the request/response cycle out of sync.
We need to move this line to where error handling happens, so that log_message is not coupled with returning responses to the editor. And this result can only be returned when rescuing errors for requests - not for notifications.
There was a problem hiding this comment.
Ah yes, I wasn't considering usage from add-ons.
Closes #516
This change allows Ruby LSP to stay alive even if an error occurred when trying to connect to the database.
I also added a specific
rescueforActiveRecord::NoDatabaseErrorsince this could be a common problem. Without that, users would see a very long stacktrace in the logs.It may partially help with #521 too.