(GH-11) Properly implement a STDIO transport server

Previously the transport layer was refactored but did not implement a STDIO
server.  This commit

* Concretely implements a STDIO connection.  The old references to a socket are
  removed

* A new SimpleSTDIOServer class was created which implements a blocking STDIO
  pipe reader.  It also implements an async stop method which be called when
  the Language Server wishes to exit.

* The server disables verbose logging (e.g. Kernel.warn) which would contaminate
  STDIO streams

* The server uses binary mode which stops ruby being helpful and injecting CRLF
  unnecessarily.

* Adds a logging destination which diverts any puppet logging into the
  language server log otherwise this would contaminate the STDIO streams

CHANGELOG.md

@@ -6,7 +6,8 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how
 
 ## Unreleased
 
-- ([GH-11](https://github.com/lingua-pupuli/puppet-editor-services/issues/11))  Refactor the transport layers to loosen object coupling
+- ([GH-11](https://github.com/lingua-pupuli/puppet-editor-services/issues/11)) Refactor the transport layers to loosen object coupling
+- ([GH-11](https://github.com/lingua-pupuli/puppet-editor-services/issues/11)) Fix STDIO server
 
 ## 0.10.0 - 2018-03-29
 

lib/puppet-editor-services/simple_stdio_server.rb

@@ -1,26 +1,91 @@
 module PuppetEditorServices
   class SimpleSTDIOServerConnection < SimpleServerConnectionBase
-    attr_accessor :socket
+    attr_accessor :simple_stdio_server
 
-    def initialize(socket)
-      @socket = socket
+    def initialize(simple_stdio_server)
+      @simple_stdio_server = simple_stdio_server
     end
 
     def send_data(data)
-      return false if socket.nil?
-      socket.write(data)
+      $stdout.write(data)
       true
     end
 
     def close_connection_after_writing
-      socket.flush unless socket.nil?
-      simple_tcp_server.remove_connection_async(socket)
+      $stdout.flush
+      @simple_stdio_server.close_connection
       true
     end
 
     def close_connection
-      simple_tcp_server.remove_connection_async(socket)
+      @simple_stdio_server.close_connection
       true
     end
   end
+
+  class SimpleSTDIOServer
+    attr_accessor :exiting
+
+    def log(message)
+      PuppetEditorServices.log_message(:debug, "STDIOSRV: #{message}")
+    end
+
+    def initialize
+      @exiting = false
+    end
+
+    def start(handler_klass = PuppetEditorServices::SimpleTCPServerConnection, connection_options = {})
+      connection_options[:servicename] = 'LANGUAGE SERVER' if connection_options[:servicename].nil?
+      # This is a little heavy handed but we need to suppress writes to STDOUT and STDERR
+      $VERBOSE = nil
+
+      $stdout.sync = true
+      # Stop the stupid CRLF injection when on Windows
+      $stdout.binmode unless $stdout.binmode
+
+      handler = handler_klass.new(connection_options)
+      client_connection = PuppetEditorServices::SimpleSTDIOServerConnection.new(self)
+      handler.client_connection = client_connection
+      handler.post_init
+
+      log('Starting STDIO server...')
+      loop do
+        inbound_data = nil
+        read_from_pipe($stdin, 2) { |data| inbound_data = data }
+        break if @exiting
+        handler.receive_data(inbound_data) unless inbound_data.nil?
+        break if @exiting
+      end
+      log('STDIO server stopped')
+    end
+
+    def stop
+      log('Stopping STDIO server...')
+      @exiting = true
+    end
+
+    def close_connection
+      stop
+    end
+
+    def pipe_is_readable?(stream, timeout = 0.5)
+      read_ready = IO.select([stream], [], [], timeout)
+      read_ready && stream == read_ready[0][0]
+    end
+
+    def read_from_pipe(pipe, timeout = 0.1, &_block)
+      if pipe_is_readable?(pipe, timeout)
+        l = nil
+        begin
+          l = pipe.readpartial(4096)
+        rescue # rubocop:disable Style/RescueStandardError, Lint/HandleExceptions
+          # Any errors here should be swallowed because the pipe could be in any state
+        end
+        # since readpartial may return a nil at EOF, skip returning that value
+        # client_connected = true unless l.nil?
+        yield l unless l.nil?
+      end
+      nil
+    end
+  end
 end

lib/puppet-languageserver.rb

@@ -130,6 +130,10 @@ def self.init_puppet(options)
   def self.init_puppet_worker(options)
     options[:puppet_settings].nil? ? Puppet.initialize_settings : Puppet.initialize_settings(options[:puppet_settings])
 
+    # Remove all other logging destinations except for ours
+    Puppet::Util::Log.destinations.clear
+    Puppet::Util::Log.newdestination('null_logger')
+
     log_message(:info, "Using Facter v#{Facter.version}")
     if options[:preload_puppet]
       log_message(:info, 'Preloading Puppet Types (Sync)...')
@@ -150,27 +154,18 @@ def self.init_puppet_worker(options)
 
   def self.rpc_server(options)
     log_message(:info, 'Starting RPC Server...')
+    options[:servicename] = 'LANGUAGE SERVER'
 
     if options[:stdio]
-      $stdin.sync = true
-      $stdout.sync = true
-
-      handler = PuppetLanguageServer::JSONRPCHandler.new(options)
-      client_connection = PuppetEditorServices::SimpleSTDIOServerConnection.new($stdout)
-      handler.client_connection = client_connection
-      handler.post_init
-
-      loop do
-        data = $stdin.readpartial(1048576)
-        raise 'Receieved an empty input string' if data.length.zero?
+      log_message(:debug, 'Using STDIO')
+      server = PuppetEditorServices::SimpleSTDIOServer.new
 
-        handler.receive_data(data)
-      end
+      trap('INT') { server.stop }
+      server.start(PuppetLanguageServer::JSONRPCHandler, options)
     else
+      log_message(:debug, 'Using Simple TCP')
       server = PuppetEditorServices::SimpleTCPServer.new
 
-      options[:servicename] = 'LANGUAGE SERVER'
-
       server.add_service(options[:ipaddress], options[:port])
       trap('INT') { server.stop_services(true) }
       server.start(PuppetLanguageServer::JSONRPCHandler, options, 2)

lib/puppet-languageserver/puppet_monkey_patches.rb

@@ -57,3 +57,11 @@ def newtype(name, options = {}, &block)
     end
   end
 end
+
+# MUST BE LAST!!!!!!
+# Suppress any warning messages to STDOUT.  It can pollute stdout when running in STDIO mode
+Puppet::Util::Log.newdesttype :null_logger do
+  def handle(msg)
+    PuppetLanguageServer.log_message(:debug, "[PUPPET LOG] [#{msg.level}] #{msg.message}")
+  end
+end