Re-factor the BaseException.name handling, and clean-up some code
#13886
Conversation
Once we're finally able to get rid of SystemJS, which is unfortunately still blocked on [bug 1247687](https://bugzilla.mozilla.org/show_bug.cgi?id=1247687), we might also want to clean-up (or even completely remove) the `BaseException` abstraction and simply extend `Error` directly instead. At that point we'd need to (explicitly) set the `name` on each class anyway, so this patch is essentially preparing for future clean-up. Furthermore, after the `BaseException` abstraction was added there's been *multiple* issues filed about third-party minification breaking our code since `this.constructor.name` is not guaranteed to always do what you intended. While hard-coding the strings indeed feels quite unfortunate, it's likely the "best" solution to avoid the problem described above.
The original idea behind including the class name, when logging errors, was to improve things in the *hypothetical case* where `PDFViewer`- and `PDFSinglePageViewer`-instances would be used side-by-side. Given that all of the relevant methods are synchronous this seem unlikely to really be necessary, and furthermore it's probably best to avoid using `this.constructor.name` since that's not guaranteed to do what you intend (we've seen repeated issues with minifiers mangling function/class names).
…Parameters` While not really relevant to the previous patches, this fixes a small inconsistency in the code.
Snuffleupagus
changed the title
BaseException.name handling, and clean-up some code
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/6c7ece30b581acf/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/844901a3387314b/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/844901a3387314b/output.txt Total script time: 33.66 mins
Image differences available at: http://54.67.70.0:8877/844901a3387314b/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/6c7ece30b581acf/output.txt Total script time: 38.99 mins
Image differences available at: http://3.101.106.178:8877/6c7ece30b581acf/reftest-analyzer.html#web=eq.log |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/d8d445d1fc0ac27/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/d8d445d1fc0ac27/output.txt Total script time: 5.18 mins Published |
|
Looks good to me; thanks! |
Please refer to the individual commit messages for additional details.