Wrapped localstorage.setItem in try block to avoid uncaught exceptions. #4826
Conversation
|
We were thinking to avoid that (since Promise catches that for us) and handle that in the user of the promise. |
|
At lines 85 and 135, we shall do something like |
|
Done |
| try { | ||
| localStorage.setItem('database', databaseStr); | ||
| } catch (ex) { | ||
| reject(); |
There was a problem hiding this comment.
it shall do that automatically since it's inside Promise(function () {...})
There was a problem hiding this comment.
Could you revert back this and the cache above (remove try/catch)?
There was a problem hiding this comment.
When I remove the try/catch, the exception is not caught and I still get the error in the console. You think it can be an issue in the Promise implementation?
There was a problem hiding this comment.
We shall investigate that, yes. The following works as expected in FF:
new Promise(function () { throw new Error(); }).then(null, function(e) { console.log('Hi'); }).then(function () { console.log('bye'); })
prints 'hi', 'bye'. We might have a bad polyfill.
|
(just fyi, there is also |
|
I understand the need to make changes to However, do we really need to change any code in Preferences.set('prefName', prefValue).then(function resolved(value) { }, function rejected(reason) { }) |
|
Really nice. (Could you also check if native implementation has the 'catch' as well, I think some older implementations might not have it) Is exception raising in the Promise resolver still a problem in the polyfill version? |
|
I'm thinking having |
|
The exception is indeed still raised. I will look into the Promise polyfill. |
|
@dferer is reverting preferences.js changes be bad in your case? I think we want to keep preferences failing |
|
Thanks, it works |
|
No I only changed preferences.js because I looked for all occurrences of localstorage,setItem. But I don't have a problem with it. I will revert it (I cannot find any occurrence of Preferences.set though). |
Fixed Promise so it rejects on uncaught exception Catch possible rejection on ViewHistory.setMultiple
|
Done |
|
That's awesome. Thanks. /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/7aff24b1768dd15/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/ecdeba655e8ae7f/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/ecdeba655e8ae7f/output.txt Total script time: 2.90 mins
Image differences available at: http://107.22.172.223:8877/ecdeba655e8ae7f/reftest-analyzer.html#web=eq.log |
|
/botio-windows test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/2f4e119eceee480/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/7aff24b1768dd15/output.txt Total script time: 26.67 mins
|
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/2f4e119eceee480/output.txt Total script time: 25.80 mins
|
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/feb38605d8ef96d/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/feb38605d8ef96d/output.txt Total script time: 0.72 mins Published
|
Wrapped localstorage.setItem in try block to avoid uncaught exceptions.
|
Thank you for the patch |
ghost
deleted the
Example: QuotaExceededError (iOS simulators)