This PR intends to add unit testing, clean up some documentation, make debugging easier for users, and improve code readability in certain areas. It was important when making these changes to make sure that none of these changes would break the existing code that users have written while using this library, and I believe I've succeeded with that. There are a lot of changes, so I'd love to hear your feedback. I tried to make sure that I documented all the changes in this PR, and I attempted to break it up into multiple small commits to make changes easier to review. However, if I have missed anything or if you have any questions, please let me know.
Changes
Authentication.ts
Updated imports to only import the exec and promisfy functions instead of the entire modules. This should make the code a bit more efficient and easier to read.
Updated the declaration of exec to match the new imports.
Added a new ProcessArgsParsingError class for when there's an issue with the regex matching. Included public rawStdout, port, password, and pid properties to the class for easier debugging.
Clarified that tryAuthenticate() returns a promise that resolves to the Credentials object.
Removed an unnecessary else statement in authenticate().
Separated getting the command line arguments and matching them into two separate functions: getProcessArgs() and parseProcessArgs(). Both functions are exported; however, they are marked with @internal and are not exported in index.ts, so TypeScript won't export their definitions.
parseProcessArgs()
Set three arguments for the function: rawStdout, which should contain the command line arguments; unsafe, a boolean that determines if the user wants to connect in unsafe mode (with an undefined certificate), which defaults to false; and certificate, an optional string that contains a user provided certificate.
Added |$ to the end of each regex to ensure that if the port, password, or pid is the last argument, it still matches properly.
Changed from array destructuring as it would throw an error if there was no match. Now, the variable will be set to undefined if there's no match.
Added a check to ensure that the port, password, and pid are all defined and the correct type before returning the object. If not, it will throw the new ProcessArgsParsingError error.
Attempted to simplify the code for the certificate flow and added more comments to explain what's going on.
Authentication.test.ts
Made an expected credentials object that is used in most new tests and is reset before each test.
Added tests to ensure that the getProcessArgs() function and regex inside of it works as expected. Those tests use different stdout formats that have been seen in issues. The new tests are:
Command line arguments separated by spaces.
Command line arguments surrounded by quotes.
Including symbols in the auth token.
Using no certificate/unsafe mode.
Using a custom certificate which in this case is the default certificate without the first line.
Invalid command line arguments. The error class should return what was matched and the stdout that was passed in.
Note: PLAINTEXT_CERT doesn't perfectly match the default certificate as it's missing a new line at the beginning. I'm unsure if this was intentional, but I used that difference to do testing with a different certificate.
Webhook.ts
Added an internal ErrorCode enum. This is used by the error handler to relay what type of error occurred and what action to take.
Removed options.__internalMockFaultyConnection as it is no longer needed for testing.
Added a new LeagueWebSocketInitError class for when there's an issue initializing the websocket. The original error event is passed using the errorEvent property. Although this is a long class name, I felt it was important to clarify that this error happens during initialization and not after it's connected.
Changed the https.Agent ternary operator syntax in an attempt to make it more readable.
Extracted the error handler into its own function, errorHandler(), to assist in easier testing. This function is exported; however, it is also marked with @internal, so TypeScript won't export it's definition.
errorHandler() has two arguments for the function. err accepts the error thrown by the webhook and options which is the options the user provided when creating the webhook. Returns a ErrorCode enum value.
ws.onerror was updated to call the new errorHandler() function and pass the error and options. The return value is then used to determine what action to take.
Webhook.test.ts
Created a MockWebSocket class that extends the EventEmitter class. This class is used to mock the websocket connection errors.
Added beforeEach function to reset the mock callback and websocket before each test.
Added a new set of tests that do not require the client to be running, but if the client is running, it will not interfere with the tests. Added tests are:
Handling an ECONNREFUSED error with maxRetries set to 0.
Handling an ECONNREFUSED error with maxRetries set to 3 and pollInterval set to 500. This test is truly run 4 times as the first attempt is not considered a retry.
Handling a different error. Unknown is used for this test, but it could be any error.
Changed the test for creating a websocket after multiple retries since __internalMockFaultyConnection was removed.
Added a test to ensure the error handler works with the real websocket. This is used to verify the ws.onerror part of the code works as expected.
Client.test.ts
Fixed a timeout that was causing the tests not to close properly.
Description
This PR intends to add unit testing, clean up some documentation, make debugging easier for users, and improve code readability in certain areas. It was important when making these changes to make sure that none of these changes would break the existing code that users have written while using this library, and I believe I've succeeded with that. There are a lot of changes, so I'd love to hear your feedback. I tried to make sure that I documented all the changes in this PR, and I attempted to break it up into multiple small commits to make changes easier to review. However, if I have missed anything or if you have any questions, please let me know.
Changes
Authentication.ts
exec
andpromisfy
functions instead of the entire modules. This should make the code a bit more efficient and easier to read.exec
to match the new imports.ProcessArgsParsingError
class for when there's an issue with the regex matching. Included publicrawStdout
,port
,password
, andpid
properties to the class for easier debugging.tryAuthenticate()
returns a promise that resolves to theCredentials
object.authenticate()
.getProcessArgs()
andparseProcessArgs()
. Both functions are exported; however, they are marked with@internal
and are not exported inindex.ts
, so TypeScript won't export their definitions.parseProcessArgs()
rawStdout
, which should contain the command line arguments;unsafe
, a boolean that determines if the user wants to connect in unsafe mode (with an undefined certificate), which defaults to false; andcertificate
, an optional string that contains a user provided certificate.|$
to the end of each regex to ensure that if the port, password, or pid is the last argument, it still matches properly.ProcessArgsParsingError
error.Authentication.test.ts
getProcessArgs()
function and regex inside of it works as expected. Those tests use different stdout formats that have been seen in issues. The new tests are:Note:
PLAINTEXT_CERT
doesn't perfectly match the default certificate as it's missing a new line at the beginning. I'm unsure if this was intentional, but I used that difference to do testing with a different certificate.Webhook.ts
ErrorCode
enum. This is used by the error handler to relay what type of error occurred and what action to take.options.__internalMockFaultyConnection
as it is no longer needed for testing.LeagueWebSocketInitError
class for when there's an issue initializing the websocket. The original error event is passed using theerrorEvent
property. Although this is a long class name, I felt it was important to clarify that this error happens during initialization and not after it's connected.errorHandler()
, to assist in easier testing. This function is exported; however, it is also marked with@internal
, so TypeScript won't export it's definition.errorHandler()
has two arguments for the function.err
accepts the error thrown by the webhook andoptions
which is the options the user provided when creating the webhook. Returns a ErrorCode enum value.ws.onerror
was updated to call the newerrorHandler()
function and pass the error and options. The return value is then used to determine what action to take.Webhook.test.ts
MockWebSocket
class that extends theEventEmitter
class. This class is used to mock the websocket connection errors.maxRetries
set to 0.maxRetries
set to 3 andpollInterval
set to 500. This test is truly run 4 times as the first attempt is not considered a retry.Unknown
is used for this test, but it could be any error.__internalMockFaultyConnection
was removed.ws.onerror
part of the code works as expected.Client.test.ts
index.ts
ClientElevatedPermsError
class.ProcessArgsParsingError
andLeagueWebSocketInitError
to the export.