castwide / vscode-solargraph

A Visual Studio Code extension for Solargraph.
Other
423 stars 25 forks source link

Formatting a file with certain characters excepts the connected language server; Encoding::InvalidByteSequenceError #166

Closed Fire- closed 3 years ago

Fire- commented 4 years ago

Given a file containing:

unicodetest.rb

"test"
"No Coverage?"
"⚠️ No Coverage?"

Download: unicodetest.rb.zip

Formatting or saving ( with formatting enabled ) causes an exception in the connected language server:

console.ts:137 [Extension Host] [INFO] Server received textDocument/formatting
                                [DEBUG] {"jsonrpc"=>"2.0", "id"=>26, "method"=>"textDocument/formatting", "params"=>{"textDocument"=>{"uri"=>"file:///Users/fire/projects/unicodetest.rb"}, "options"=>{"tabSize"=>2, "insertSpaces"=>true}}}
console.ts:137 [Extension Host] [INFO] Cataloging /Users/fire/projects
console.ts:137 [Extension Host] [INFO] Catalog complete (84785 pins)
console.ts:137 [Extension Host] [INFO] Sending response to textDocument/formatting
console.ts:137 [Extension Host] [DEBUG] {:jsonrpc=>"2.0", :id=>26, :result=>[{:range=>{:start=>{:line=>0, :character=>0}, :end=>{:line=>2, :character=>17}}, :newText=>"\"test\"\n\"No Coverage?\"\n\"\xE2\x9A\xA0\xEF\xB8\x8F No Coverage?\""}]}
console.ts:137 [Extension Host] #<Thread:0x00007f92730800e0@/Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/backport-1.1.2/lib/backport/client.rb:101 run> terminated with exception (report_on_exception is true):
console.ts:137 [Extension Host] /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/message/base.rb:77:in `encode': "\xE2" on US-ASCII (Encoding::InvalidByteSequenceError)
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/message/base.rb:77:in `to_json'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/message/base.rb:77:in `send_response'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/adapter.rb:45:in `process'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/adapter.rb:17:in `block in opening'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/data_reader.rb:57:in `parse_message_from_buffer'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/data_reader.rb:35:in `block in receive'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/data_reader.rb:30:in `each_char'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/data_reader.rb:30:in `receive'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/lib/solargraph/language_server/transport/adapter.rb:27:in `receiving'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/backport-1.1.2/lib/backport/client.rb:63:in `tick'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/backport-1.1.2/lib/backport/server/tcpip.rb:76:in `update'
                           from /Users/fire/.rbenv/versions/2.5.1/lib/ruby/2.5.0/observer.rb:197:in `block in notify_observers'
                           from /Users/fire/.rbenv/versions/2.5.1/lib/ruby/2.5.0/observer.rb:196:in `each'
                           from /Users/fire/.rbenv/versions/2.5.1/lib/ruby/2.5.0/observer.rb:196:in `notify_observers'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/backport-1.1.2/lib/backport/client.rb:121:in `read_input'
                           from /Users/fire/projects/vendor/bundle/ruby/2.5.0/gems/backport-1.1.2/lib/backport/client.rb:102:in `block in run_input_thread'

This worked fine "recently", and I'm not entirely sure what caused the regression -- as it appears that this was fixed multiple times prior -- or when, exactly, either. I simply noticed mid-project that my saves were spinning due to solargraph as a format participant, but couldn't locate a source ( as the solargraph server itself continues to run, but no longer responds to messages )

I do not believe this occurred with 0.38.4, but occurs with 0.38.5 ( first noticed ), and 0.38.6 ( updated to verify not fixed )

A "quick" fix seems to be enforcing the default external encoding before an entrypoint, such as in bin/solargraph:

vendor/bundle/ruby/2.5.0/gems/solargraph-0.38.6/bin/solargraph

#!/usr/bin/env ruby

Encoding.default_external = 'UTF-8'
require 'solargraph'

Solargraph::Shell.start(ARGV)

However, I'm unsure if that's really the correct way of correcting for something like this.

castwide commented 4 years ago

I don't get an error, but formatting escapes unicode characters inside strings:

# frozen_string_literal: true

'test'
'No Coverage?'
"\xE2\x9A\xA0\xEF\xB8\x8F No Coverage?"

What version of RuboCop are you using? You can confirm which one is loaded in Solargraph by running Solargraph: Get environment info from the command palette. My setup uses 0.80.1.

Fire- commented 4 years ago

The project we're on is legacy, so currently:

System
Platform: x86_64-darwin18
Shell: /usr/local/bin/bash
Ruby Version: 2.5.1
RuboCop Version: 0.55.0

Solargraph Version: 0.38.6
Core Documentation Version: 2.5.1
Core Cache Directory: /Users/fire/.solargraph/cache
Parser Target Version: 25
Using Bundler: true

platform/shell change ( for rubocup ) when running via docker ( locally / in CI )

castwide commented 4 years ago

I tried again using RuboCop 0.55.0, but haven't been able to reproduce this error.

What setting do you have for files.encoding in VS Code? (Mine is utf-8.)

Fire- commented 4 years ago

Hope you had a good weekend! files.encoding is utf8 here as well

Fire- commented 4 years ago

@castwide I'm unsure if it's directly related, but I managed to cause this same error today by complete accident by copying the output of Pry from VSCode's terminal into a file and attempting to save. It caused the exception and then the server itself spun indefinitely until I restarted the extension host manually.

`encode': "\xC3" on US-ASCII (Encoding::InvalidByteSequenceError)

I then managed to cause it again by selecting the copied output itself and attempting to format it

`encode': "\xE2" on US-ASCII (Encoding::InvalidByteSequenceError)

just like as noted in the original report, adding Encoding.default_external = 'UTF-8' to the gem's binary in vendor/bundle/ruby/2.5.0/gems/solargraph-0.39.7/bin/solargraph resolved the issue ( I'd lost it since I had updated )

New env info

System
Platform: x86_64-darwin18
Shell: /usr/local/bin/bash
Ruby Version: 2.5.1
RuboCop Version: 0.55.0

Solargraph
Solargraph Version: 0.39.7
Core Documentation Version: 2.5.1
Core Cache Directory: /Users/fire/.solargraph/cache
Parser Target Version: 25
Using Bundler: true
castwide commented 4 years ago

@Fire- Thanks for tracking that down and reminding me about Encoding.default_external. I'll add that in the next patch.

castwide commented 4 years ago

The latest version of the gem (0.39.11) sets Encoding.default_external.