Skip to content

Commit 5ac34cf

Browse files
committed
fix: wrap ERROR and WARN logging in vim.schedule to prevent fast event context errors
Fixes issue where closing and reopening MacBook would trigger TCP connection errors that call logger.error() from timer callbacks (fast event context), causing "nvim_echo must not be called in a fast event context" errors. Changes: - Wrap vim.notify calls for ERROR and WARN levels in vim.schedule() - Add comprehensive test suite for logger fast event context safety - All logging levels now safely callable from any event context Fixes #51 Change-Id: I9791d9e788745225aeac7c19c6c33562f95f086b Signed-off-by: Thomas Kosiewski <[email protected]>
1 parent db91a0a commit 5ac34cf

File tree

2 files changed

+181
-2
lines changed

2 files changed

+181
-2
lines changed

lua/claudecode/logger.lua

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,13 @@ local function log(level, component, message_parts)
6969
end
7070

7171
if level == M.levels.ERROR then
72-
vim.notify(prefix .. " " .. message, vim.log.levels.ERROR, { title = "ClaudeCode Error" })
72+
vim.schedule(function()
73+
vim.notify(prefix .. " " .. message, vim.log.levels.ERROR, { title = "ClaudeCode Error" })
74+
end)
7375
elseif level == M.levels.WARN then
74-
vim.notify(prefix .. " " .. message, vim.log.levels.WARN, { title = "ClaudeCode Warning" })
76+
vim.schedule(function()
77+
vim.notify(prefix .. " " .. message, vim.log.levels.WARN, { title = "ClaudeCode Warning" })
78+
end)
7579
else
7680
-- For INFO, DEBUG, TRACE, use nvim_echo to avoid flooding notifications,
7781
-- to make them appear in :messages, and wrap in vim.schedule

tests/unit/logger_spec.lua

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
-- luacheck: globals expect
2+
require("tests.busted_setup")
3+
4+
describe("Logger", function()
5+
local logger
6+
local original_vim_schedule
7+
local original_vim_notify
8+
local original_nvim_echo
9+
local scheduled_calls = {}
10+
local notify_calls = {}
11+
local echo_calls = {}
12+
13+
local function setup()
14+
package.loaded["claudecode.logger"] = nil
15+
16+
-- Mock vim.schedule to track calls
17+
original_vim_schedule = vim.schedule
18+
vim.schedule = function(fn)
19+
table.insert(scheduled_calls, fn)
20+
-- Immediately execute the function for testing
21+
fn()
22+
end
23+
24+
-- Mock vim.notify to track calls
25+
original_vim_notify = vim.notify
26+
vim.notify = function(msg, level, opts)
27+
table.insert(notify_calls, { msg = msg, level = level, opts = opts })
28+
end
29+
30+
-- Mock nvim_echo to track calls
31+
original_nvim_echo = vim.api.nvim_echo
32+
vim.api.nvim_echo = function(chunks, history, opts)
33+
table.insert(echo_calls, { chunks = chunks, history = history, opts = opts })
34+
end
35+
36+
logger = require("claudecode.logger")
37+
38+
-- Set log level to TRACE to enable all logging levels for testing
39+
logger.setup({ log_level = "trace" })
40+
end
41+
42+
local function teardown()
43+
vim.schedule = original_vim_schedule
44+
vim.notify = original_vim_notify
45+
vim.api.nvim_echo = original_nvim_echo
46+
scheduled_calls = {}
47+
notify_calls = {}
48+
echo_calls = {}
49+
end
50+
51+
before_each(function()
52+
setup()
53+
end)
54+
55+
after_each(function()
56+
teardown()
57+
end)
58+
59+
describe("error logging", function()
60+
it("should wrap error calls in vim.schedule", function()
61+
logger.error("test", "error message")
62+
63+
-- Should have made one scheduled call
64+
expect(#scheduled_calls).to_be(1)
65+
66+
-- Should have called vim.notify with error level
67+
expect(#notify_calls).to_be(1)
68+
expect(notify_calls[1].level).to_be(vim.log.levels.ERROR)
69+
assert_contains(notify_calls[1].msg, "error message")
70+
end)
71+
72+
it("should handle error calls without component", function()
73+
logger.error("error message")
74+
75+
expect(#scheduled_calls).to_be(1)
76+
expect(#notify_calls).to_be(1)
77+
assert_contains(notify_calls[1].msg, "error message")
78+
end)
79+
end)
80+
81+
describe("warn logging", function()
82+
it("should wrap warn calls in vim.schedule", function()
83+
logger.warn("test", "warning message")
84+
85+
-- Should have made one scheduled call
86+
expect(#scheduled_calls).to_be(1)
87+
88+
-- Should have called vim.notify with warn level
89+
expect(#notify_calls).to_be(1)
90+
expect(notify_calls[1].level).to_be(vim.log.levels.WARN)
91+
assert_contains(notify_calls[1].msg, "warning message")
92+
end)
93+
94+
it("should handle warn calls without component", function()
95+
logger.warn("warning message")
96+
97+
expect(#scheduled_calls).to_be(1)
98+
expect(#notify_calls).to_be(1)
99+
assert_contains(notify_calls[1].msg, "warning message")
100+
end)
101+
end)
102+
103+
describe("info logging", function()
104+
it("should wrap info calls in vim.schedule", function()
105+
logger.info("test", "info message")
106+
107+
-- Should have made one scheduled call
108+
expect(#scheduled_calls).to_be(1)
109+
110+
-- Should have called nvim_echo instead of notify
111+
expect(#echo_calls).to_be(1)
112+
expect(#notify_calls).to_be(0)
113+
assert_contains(echo_calls[1].chunks[1][1], "info message")
114+
end)
115+
end)
116+
117+
describe("debug logging", function()
118+
it("should wrap debug calls in vim.schedule", function()
119+
logger.debug("test", "debug message")
120+
121+
-- Should have made one scheduled call
122+
expect(#scheduled_calls).to_be(1)
123+
124+
-- Should have called nvim_echo instead of notify
125+
expect(#echo_calls).to_be(1)
126+
expect(#notify_calls).to_be(0)
127+
assert_contains(echo_calls[1].chunks[1][1], "debug message")
128+
end)
129+
end)
130+
131+
describe("trace logging", function()
132+
it("should wrap trace calls in vim.schedule", function()
133+
logger.trace("test", "trace message")
134+
135+
-- Should have made one scheduled call
136+
expect(#scheduled_calls).to_be(1)
137+
138+
-- Should have called nvim_echo instead of notify
139+
expect(#echo_calls).to_be(1)
140+
expect(#notify_calls).to_be(0)
141+
assert_contains(echo_calls[1].chunks[1][1], "trace message")
142+
end)
143+
end)
144+
145+
describe("fast event context safety", function()
146+
it("should not call vim API functions directly", function()
147+
-- Simulate a fast event context by removing the mocked functions
148+
-- and ensuring no direct calls are made
149+
local direct_notify_called = false
150+
local direct_echo_called = false
151+
152+
vim.notify = function()
153+
direct_notify_called = true
154+
end
155+
156+
vim.api.nvim_echo = function()
157+
direct_echo_called = true
158+
end
159+
160+
vim.schedule = function(fn)
161+
-- Don't execute the function, just verify it was scheduled
162+
table.insert(scheduled_calls, fn)
163+
end
164+
165+
logger.error("test", "error in fast context")
166+
logger.warn("test", "warn in fast context")
167+
logger.info("test", "info in fast context")
168+
169+
-- All should be scheduled, none should be called directly
170+
expect(#scheduled_calls).to_be(3)
171+
expect(direct_notify_called).to_be_false()
172+
expect(direct_echo_called).to_be_false()
173+
end)
174+
end)
175+
end)

0 commit comments

Comments
 (0)