[Fix #11034] Fix hang when using --stderr in server mode

[tdeo]

I was able to write a reproduction test case for this bug.

It seemed a bit more intricate than the original issue description as the hanging seem to only happen when cache is at play (hence having to run the command twice in the reproduction test).

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[koic]

I'm not sure whether it's the better solution for the issue. This patch solves the unexpected pause issue, but --stderr behavior is still not as expected.

--stderr has the following feature:

% bundle exec rubocop -h
(snip)
        --stderr                     Write all output to stderr except for the
                                     autocorrected source. This is especially useful
                                     when combined with --autocorrect and --stdin.

Below is a run without server mode, but same results would have been expected with server mode:

% cat example.rb
puts "hello"

% bundle exec rubocop --stderr example.rb -a 2> /tmp/11034.log

% cat /tmp/11034.log
(snip)
RSpec/Rails/AvoidSetupHook: # new in 2.4
  Enabled: true
RSpec/Rails/HaveHttpStatus: # new in 2.12
  Enabled: true
For more information: https://docs.rubocop.org/rubocop/versioning.html
Inspecting 1 file
C

Offenses:

example.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
puts 'hello'
^
example.rb:1:6: C: [Corrected] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
puts "hello"
     ^^^^^^^

1 file inspected, 2 offenses detected, 1 offense corrected, 1 more offense can be corrected with `rubocop -A`

Unexpected behavior still displays in the console when the server is enabled:

% bundle exec rubocop --start-server
RuboCop server starting on 127.0.0.1:50281.

% bundle exec rubocop --stderr example.rb -a 2> /tmp/11034.log
(snip)
RSpec/Rails/AvoidSetupHook: # new in 2.4
  Enabled: true
RSpec/Rails/HaveHttpStatus: # new in 2.12
  Enabled: true
For more information: https://docs.rubocop.org/rubocop/versioning.html
Inspecting 1 file
C

Offenses:

example.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
puts 'hello'
^
example.rb:1:6: C: [Corrected] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
puts "hello"
     ^^^^^^^

1 file inspected, 2 offenses detected, 1 offense corrected, 1 more offense can be corrected with `rubocop -A`

Even with server mode enabled, it would be expected to be redirected as stderr.

[tdeo]

@koic I think I managed to find a working implementation for combining --stderr with --server and have only the corrected source showing on stdout and the rest on stderr.

I otherwise considered sending both streams through the socket by using a dedicated separation line (in the likes of ====== BEGIN STDERR =====) instead of using a file to store the output - let me know if you think that would be a better approach

[koic]

I have confirmed that the redirected file is empty due to an unexpected behavior.

% bundle exec rubocop --start-server
RuboCop server starting on 127.0.0.1:64200.
% bundle exec rubocop --stderr example.rb -a 2> /tmp/11034.log
% cat /tmp/11034.log # empty

Can you check and add a test for the above case?

[tdeo]

stderr output was indeed missing because I had accidentally deleted https://github.com/rubocop/rubocop/pull/11060/files#diff-f3ac671b93c53d20faa674a2f9d4fc95517b7306a1a1ac5eb7b8619a731310f9R26 while doing some cleanup - should be all good now

[tdeo]

Should I add a changelog entry about support for --stderr in --server mode since this may impact users?

[koic]

Should I add a changelog entry about support for --stderr in --server mode since this may impact users?

Yes. Please add a changelog about this fixing! 👍

[koic]

Great! Thanks!

[tdeo]

Happy to contribute 🥳

[miry]

I would like to report the error related to this changes:

if the exception happen on request = parse_request(@socket.read) https://github.com/rubocop/rubocop/pull/11060/files#diff-82975553b87a587bacd64e8f1e24aaabea34dac34a530d8355f41b26086c4c63R25,

then there is stderr variable is not defined and next problem happens in https://github.com/rubocop/rubocop/pull/11060/files#diff-82975553b87a587bacd64e8f1e24aaabea34dac34a530d8355f41b26086c4c63R36:

home/.rvm/gems/ruby-3.3.6@foo/gems/rubocop-1.66.1/lib/rubocop/server/socket_reader.rb:36:in `ensure in read!': undefined method `string' for nil (NoMethodError)

        Cache.stderr_path.write(stderr.string)
                                      ^^^^^^^
        from home/.rvm/gems/ruby-3.3.6@foo/gems/rubocop-1.66.1/lib/rubocop/server/socket_reader.rb:37:in `read!'

[miry]

In ruby 3.3.6 the stringio is extracted to default gem and it is required to require "stringio":

An error occurred while Layout/FirstArgumentIndentation cop was inspecting path/spec/services/filaments/tiraplast/sync_filament_payments_service_spec.rb:124:54.
uninitialized constant #<Class:RuboCop::Cop::AlignmentCorrector>::StringIO
path/vendor/bundle/ruby/3.3.0/gems/rubocop-1.66.1/lib/rubocop/cop/correctors/alignment_corrector.rb:101:in `remove'

I believe it is fixed in #13379

[Earlopain]

Yes, that PR should fix it. psych removed a require on stringio in ruby/psych#686 which RuboCop relied on in some places. If you have psych in your lockfile and updated it you will hit this today already.