-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Convert the various ...Exception
s to proper classes, to reduce code duplication
#11185
Conversation
...Exception
s to proper classes, to reduced code duplication...Exception
s to proper classes, to reduce code duplication
0777f63
to
2e11e2d
Compare
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/848c0e96affcbbe/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/ef26e4c37bd802f/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/ef26e4c37bd802f/output.txt Total script time: 2.61 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/848c0e96affcbbe/output.txt Total script time: 5.15 mins
|
this.name = 'PasswordException'; | ||
this.message = msg; | ||
this.code = code; | ||
const BaseException = (function BaseExceptionClosure() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once extending Error
isn't a problem any more, when SystemJS has been updated/replaced, it should be possible to simplify this even further to e.g.
class BaseException extends Error {
constructor(message) {
if (this.constructor === BaseException) {
unreachable('Cannot initialize BaseException.');
}
super(message);
this.name = this.constructor.name;
}
}
… duplication By utilizing a base "class", things become significantly simpler. Unfortunately the new `BaseException` cannot be a proper ES6 class and just extend `Error`, since the SystemJS dependency doesn't seem to play well with that. Note also that we (generally) need to keep the `name` property on the actual `...Exception` object, rather than on its prototype, since the property will otherwise be dropped during the structured cloning used with `postMessage`.
2e11e2d
to
5d93fda
Compare
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/ffaced9df7fdf54/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/9f81b958e4c51e2/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/9f81b958e4c51e2/output.txt Total script time: 2.58 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/ffaced9df7fdf54/output.txt Total script time: 4.97 mins
|
Looks much cleaner. Thanks! |
By utilizing a base "class", things become significantly simpler. Unfortunately the new
BaseException
cannot be a proper ES6 class and just extendError
, since the SystemJS dependency doesn't seem to play well with that.Note also that we (generally) need to keep the
name
property on the actual...Exception
object, rather than on its prototype, since the property will otherwise be dropped during the structured cloning used withpostMessage
.