-
Notifications
You must be signed in to change notification settings - Fork 10.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MessageHandler] Re-factor and convert the code to a proper class
#11290
[MessageHandler] Re-factor and convert the code to a proper class
#11290
Conversation
…n Promise callbacks are used, with enumeration values Given that the `isReply` property is an internal implementation detail, changing its type shouldn't be a problem. Note that by directly indicating if either data or an Error is sent, it's no longer necessary to use `in` when handling the callback.
Note that using `in` leads to unnecessary stringification of the properties, which seems completely unnecessary here. To avoid future problems from these changes the `MessageHandler.on` method will now assert, in non-`PRODUCTION`/`TESTING` builds, that it's always called with a function as expected. This patch also renames `callbacksCapabilities` to `callbackCapabilities`, note the removed "s", since using a double plural format looks a bit strange.
…arly returns When `ReadableStream` support was added to the `MessageHandler`, the `_onComObjOnMessage` function became more complex than previously. All of the nested `if`/`else if`/`else` branches are now, at least in my opinion, making some of this code a bit difficult to follow. Hence this patch, which attempts to help readability by making use of early `return`s and `Error`s. The patch also changes a couple of `var`/`let` occurences to `const`.
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/a1a4f006f6fb7e4/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/3906b907821ce7d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/3906b907821ce7d/output.txt Total script time: 18.75 mins
|
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/a1a4f006f6fb7e4/output.txt Total script time: 26.70 mins
Image differences available at: http://54.215.176.217:8877/a1a4f006f6fb7e4/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/551351febc33250/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/8e1f013bc1bc137/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/551351febc33250/output.txt Total script time: 18.60 mins
|
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/8e1f013bc1bc137/output.txt Total script time: 26.46 mins
Image differences available at: http://54.215.176.217:8877/8e1f013bc1bc137/reftest-analyzer.html#web=eq.log |
I like it. Thanks! |
This should, most likely, finish the somewhat recent round of clean-up/re-factoring of the
MessageHandler
code. For easier reviewing, I'd suggest looking at each commit separately and also ignoring white-space changes where appropriate.