-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Fix the exception propagation when rejecting workerReadyCapability #5275
Fix the exception propagation when rejecting workerReadyCapability #5275
Conversation
Currently when an exception is thrown, we try to reject `workerReadyCapability` with multiple arguments in src/core/api.js. This obviously doesn't work, hence this patch changes that to instead reject with the exception object as is. In src/core/worker.js the exception is currently (unncessarily) wrapped in an object, so this patch also simplifies that to directly send the exception object instead.
…d of the exception name
…itsForPromiseResolved`and add a `waitsForPromiseRejected` function
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/15ca58dff4c1316/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/a726701694b1ca6/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/15ca58dff4c1316/output.txt Total script time: 0.66 mins
|
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/a726701694b1ca6/output.txt Total script time: 0.82 mins
|
/botio-windows preview |
From: Bot.io (Windows)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/356f308e29f0d4e/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/356f308e29f0d4e/output.txt Total script time: 0.94 mins Published
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/83b41aea9c757a1/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/d6774e5576fabde/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/d6774e5576fabde/output.txt Total script time: 20.10 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/83b41aea9c757a1/output.txt Total script time: 22.46 mins
|
I found yet another case of incorrect exception throwing, that I previously missed, so I've added another commit to address this. @yurydelendik Anything else you want me to change in this PR? |
…file loading fails because the server responds with a non 404 status message
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/318170c12c53fac/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/64ff34d8e8d709a/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/318170c12c53fac/output.txt Total script time: 3.05 mins
Image differences available at: http://107.22.172.223:8877/318170c12c53fac/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/64ff34d8e8d709a/output.txt Total script time: 22.57 mins
|
Fix the exception propagation when rejecting workerReadyCapability
Thank you for the patch |
Currently when an exception is thrown, we try to reject
workerReadyCapability
with multiple arguments in src/core/api.js. This obviously doesn't work, hence this PR changes that to instead reject with the exception object as is.In src/core/worker.js the exception is currently (unncessarily) wrapped in an object, so this patch also simplifies that to directly send the exception object instead.
Also, this PR adds a unit test to check that the exception handling is working as expected (it fails with the current
master
).Note: Two commits were extracted from PR #5234, since I think it makes sense to get the exception handling fixed as soon as possible!