(GH-11) Refactor transport layer and fix STDIO server #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ling Previously the different transport types, TCP and STDIO, were very closely coupled which made debugging and maintaining them difficult. This commit changes the relationships between the server, client connection, message handler and message router classes/objects to be much looser and can therefore be defined/instantiated easier at run time; New object model - (Transport Server) creates a (Client Connection object) per incoming connection - Each (Client Connection object) creates a (Handler object) - Each (Handler object) creates a (Message Router object) Therefore the Message Router can create messages which traverse back down the object chain and is eventually emitted by the Transport Server. This model should apply whether it is a STDIO or TCP transport * The JSON Handler no longer inherits the TCP Server Connection, but a generic SimpleServerConnectionHandler which has some base methods. The message router object is now assigned via the message_router methods, which can be passed in via the initializer, or defaults to an instance of the PuppetDebugServer::MessageRouter class. The client connection object is expected to implement the SimpleServerConnectionBase "interface" and passedv in via the Handler initializer. * The Message Router no longer inherits from the JSON handler, but has a json_handler method. It is assumed the Handler will set this when it creates an instance of the Router * A new SimpleSTDIOServerConnection object has been created which implements the base SimpleServerConnectionBase object. This object is the concrete implementation of connection over the STDIO transport * The TCP Server Connection now inherits from the SimpleServerConnectionBase object and tracks the originating TCP Server object and socket via the initializer * PuppetLanguageServer was modified to use the new STDIO connection type and use the JSON Handler instead of the MessageRouter * The document_type method was moved from the MessageRouter code to the DocumentStore module. This made more sense and means that we don't need to create an entire Message Router object to do a simple static document type check * The validation queue was modified with the new namespace of the document_type static method * Tests were modified for the new object hierarchy
glennsarti
force-pushed
the
refactor-transport-layer
branch
from
c482f6c
to
7d6cb57
Compare
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
Previously if an invalid logfile location was specified e.g. C:\ or a missing parent directory, the service (Language or Debug Server) would error. This is undesired as a bad log file should not cause a catastrophic failure. This commit modifies the logging so that an error occurs when the log file is created it, the error is sent to STDERR, and service continues but with logging disabled. This commit also flushes the log file on each write if needed. This is required as OS buffering makes real time debugging very difficult.
glennsarti
force-pushed
the
refactor-transport-layer
branch
from
7d6cb57
to
a601208
Compare
glennsarti
changed the title
|
Ready for merge |
jpogran
approved these changes
Apr 24, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the different transport types, TCP and STDIO, were very closely coupled which made debugging and maintaining them difficult. This commit changes the relationships between the server, client connection, message handler and message router classes/objects to be much looser and can therefore be defined/instantiated easier at run time;
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.
Previously if an invalid logfile location was specified e.g. C:\ or a missing parent directory, the service (Language or Debug Server) would error. This is undesired as a bad log file should not cause a catastrophic failure. This commit modifies the logging so that an error occurs when the log file is created it, the error is sent to STDERR, and service continues but with logging disabled.