Skip to content

Commit 99dace1

Browse files
authored
Wrap stderr in a structure log delegator (#564)
After #559, the server and its add-ons can send notifications, but regular `puts` that would normally be redirected to stderr are now lost. Trying to account for any type of print in the notifier thread would be difficult. We need to be able to match against the `\r\n\r\n` separator, so that we can know exactly the start and end of a structured message. So I propose we wrap stderr in a delegator instead. Essentially, any method invoked on `stderr` will still be redirect to it, but it gives us an opportunity to override `puts` and `print` to use structured logging, instead of just writing the strings directly to the pipe. I tested this with the Tapioca add-on and I'm able to accurately see both progress notifications and messages in the output tab.
1 parent 92795ff commit 99dace1

File tree

2 files changed

+43
-20
lines changed

2 files changed

+43
-20
lines changed

lib/ruby_lsp/ruby_lsp_rails/server.rb

+19-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
require "json"
55
require "open3"
6+
require "delegate"
67

78
module RubyLsp
89
module Rails
@@ -197,6 +198,23 @@ def execute(request, params)
197198
end
198199
end
199200

201+
class IOWrapper < SimpleDelegator
202+
def puts(*args)
203+
args.each { |arg| log("#{arg}\n") }
204+
end
205+
206+
def print(*args)
207+
args.each { |arg| log(arg.to_s) }
208+
end
209+
210+
private
211+
212+
def log(message)
213+
json_message = { method: "window/logMessage", params: { type: 4, message: message } }.to_json
214+
write("Content-Length: #{json_message.bytesize}\r\n\r\n#{json_message}")
215+
end
216+
end
217+
200218
class Server
201219
include Common
202220

@@ -218,7 +236,7 @@ def initialize(stdout: $stdout, stderr: $stderr, override_default_output_device:
218236
# # Set the default output device to be $stderr. This means that using `puts` by itself will default to printing
219237
# # to $stderr and only explicit `$stdout.puts` will go to $stdout. This reduces the chance that output coming
220238
# # from the Rails app will be accidentally sent to the client
221-
$> = $stderr if override_default_output_device
239+
$> = IOWrapper.new(@stderr) if override_default_output_device
222240

223241
@running = true
224242
end

test/ruby_lsp_rails/server_test.rb

+24-19
Original file line numberDiff line numberDiff line change
@@ -158,25 +158,6 @@ def execute(request, params)
158158
FileUtils.rm("server_addon.rb")
159159
end
160160

161-
test "prints in the Rails application or server are automatically redirected to stderr" do
162-
stdout = StringIO.new
163-
server = RubyLsp::Rails::Server.new(stdout: stdout)
164-
165-
server.instance_eval do
166-
def resolve_route_info(requirements)
167-
puts "Hello"
168-
super
169-
end
170-
end
171-
172-
_, stderr = capture_subprocess_io do
173-
server.execute("route_info", { controller: "UsersController", action: "index" })
174-
end
175-
176-
refute_match("Hello", stdout.string)
177-
assert_equal("Hello\n", stderr)
178-
end
179-
180161
test "checking for pending migrations" do
181162
capture_subprocess_io do
182163
system("bundle exec rails g migration CreateStudents name:string")
@@ -252,6 +233,30 @@ def resolve_route_info(requirements)
252233
assert_equal "Content-Length: #{expected_notification.bytesize}\r\n\r\n#{expected_notification}", @stderr.string
253234
end
254235

236+
test "regular prints become structured log messages" do
237+
original_stdout = $stdout
238+
239+
stdout = StringIO.new
240+
stderr = StringIO.new
241+
server = RubyLsp::Rails::Server.new(stdout: stdout, stderr: stderr, override_default_output_device: true)
242+
243+
server.instance_eval do
244+
def print_it!
245+
puts "hello"
246+
end
247+
end
248+
249+
T.unsafe(server).print_it!
250+
251+
assert_match("Content-Length: 70\r\n\r\n", stderr.string)
252+
assert_match(
253+
"{\"method\":\"window/logMessage\",\"params\":{\"type\":4,\"message\":\"hello\\n\"}}",
254+
stderr.string,
255+
)
256+
ensure
257+
$> = original_stdout
258+
end
259+
255260
private
256261

257262
def response

0 commit comments

Comments
 (0)