Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle nil error message for the REPL #1457

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

igorlfs
Copy link
Contributor

@igorlfs igorlfs commented Mar 1, 2025

Hello!

This fixes a small regression introduced in 5766946

Some adapters may return errors that when parsed by utils.fmt_error return nil (in spite of not being nil themselves), which throws an error in M.append.

An example is getting the value of a non-declared variable with the js-debug-adapter.

Copy link
Owner

@mfussenegger mfussenegger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, could you also update the annotations?

I also wonder if instead there should be a fallback in the __tostring implementation, then the full diff could look like this:

diff --git a/lua/dap/protocol.lua b/lua/dap/protocol.lua
index 59ccac1..23e5282 100644
--- a/lua/dap/protocol.lua
+++ b/lua/dap/protocol.lua
@@ -24,11 +24,11 @@
 ---@field message? "cancelled"|"notStopped"|string
 ---@field body? any
 
 
 ---@class dap.ErrorResponse: dap.Response
----@field message string
+---@field message? "cancelled"|"notStopped"|string
 ---@field body {error?: dap.Message}
 
 
 ---@class dap.Message
 ---@field id number
diff --git a/lua/dap/session.lua b/lua/dap/session.lua
index abc1d86..226d4c6 100644
--- a/lua/dap/session.lua
+++ b/lua/dap/session.lua
@@ -14,11 +14,11 @@ local mime_to_filetype = {
   ['text/javascript'] = 'javascript'
 }
 
 local err_mt = {
   __tostring = function(e)
-    return utils.fmt_error(e)
+    return utils.fmt_error(e) or "Undefined error"
   end,
 }
 
 
 local ns_pool = {}
diff --git a/lua/dap/utils.lua b/lua/dap/utils.lua
index a8a37ce..575dc91 100644
--- a/lua/dap/utils.lua
+++ b/lua/dap/utils.lua
@@ -1,10 +1,10 @@
 local M = {}
 
 
 ---@param err dap.ErrorResponse
----@return string
+---@return string?
 function M.fmt_error(err)
   local body = err.body or {}
   if body.error and body.error.showUser then
     local msg = body.error.format
     for key, val in pairs(body.error.variables or {}) do

Because if tostring on the err can return nil some of the other usages might also cause issues

@igorlfs igorlfs force-pushed the fix/nil-err-regression branch from 277d48c to e46a6ac Compare March 4, 2025 12:38
@igorlfs
Copy link
Contributor Author

igorlfs commented Mar 4, 2025

I also wonder if instead there should be a fallback in the __tostring implementation

That does sound like a better approach! I've applied your patch and rebased. Let me know if anything else is necessary.

@mfussenegger mfussenegger merged commit 8228cb0 into mfussenegger:master Mar 4, 2025
4 checks passed
@mfussenegger
Copy link
Owner

I also wonder if instead there should be a fallback in the __tostring implementation

Let me know if anything else is necessary.

All good, thanks

@igorlfs igorlfs deleted the fix/nil-err-regression branch March 4, 2025 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants