Closed Bug 1090867 Opened 9 years ago Closed 9 years ago
[Messages][Refactoring] Don't consider cancelled activity as unhandled error
Currently when activity request is cancelled by user (via Cancel button in activity target selection menu) we log it as "Unhandled error spawning activity" without any useful details. This pollutes log with confusing non-valuable entries. Cancelled activity case can be recognized like this "e.target.error.name === 'ActivityCanceled'" where "target" is the corresponding "MozActivity" instance. We can either put meaningful message to the log or just be silent in this case. Unfortunately there is no any strict agreement on exception name when activity is cancelled by target activity itself during a normal app flow (eg. if user taps on back button in Gallery\Video\Music without selecting anything), though all currently available built-in apps use "pick cancelled" exception name in this case.
I'm quite sure there is an existing bug for this :) But can't find it right now. Should we make it a mentored bug?
(In reply to Julien Wajsberg [:julienw] from comment #1) > I'm quite sure there is an existing bug for this :) But can't find it right > now. Ah, haven't found it from the first attempt :( Will try one more time and dupe one of them in this case. > Should we make it a mentored bug? I thought that it may involve some changes in how "Compose.requestAttachment" processes "file too large" exception to always have the same error object structure in "onerror" callback, but probably it can be mentored as well :)
Whiteboard: [sms-papercuts] → [sms-papercuts][lang=js][mentor-lang=ru]
Hello, I've put some condition in the "onerror" callback so that it doesn't log anything if the user cancel the attachment. I didn't change how "Compose.requestAttachment" processes the "file too large" exception because in the unit tests it use the same patern (calling onerror with a simple string) and I thought it could be use like that in other part of the code.
Assignee: nobody → maxime.lore
Status: NEW → ASSIGNED
Comment on attachment 8515636 [details] [review] pull request github Hey Maxime, Thanks for you contribution! I've left just two comments on Github mostly about unifying. Please set r? again once you're ready and ping me if you have any questions!
Hello, Sorry for taking this long (flashing my device and updating my linux took me a looong time), I've added some changes to the pull request and I had a problem that explained in a comment on the pull request. Bye
Comment on attachment 8515636 [details] [review] pull request github Hey Maxime, (In reply to Maxime Lo Re [:solfen] from comment #7) > Hello, > Sorry for taking this long (flashing my device and updating my linux took me > a looong time), Don't worry and always take your time if you need it :) > I've added some changes to the pull request and I had a > problem that explained in a comment on the pull request. > Bye I've replied on the GitHub + some other nits, but overall it looks good! Please set r? again, whenever you're ready. Thanks!
Okay I've done the requested changes, let me know what you think :) Bye
Comment on attachment 8515636 [details] [review] pull request github Looks good to me! Just two tiny syntax nits on Github. Please, squash your commits and once nits are fixed and Treeherder is green add "checkin-needed" into bug "Keywords: " field. Thanks a lot for contribution!
Attachment #8515636 - Flags: review?(azasypkin) → review+
It's done. Thanks a lot for all the help !
Linter failures on the Gaia Try run.
Apparently on B2G desktop *64 DOMError is not defined, this is weird since there is no problem when testing on a phone. Any idea why ?
(In reply to Maxime Lo Re [:solfen] from comment #13) > Apparently on B2G desktop *64 DOMError is not defined, this is weird since > there is no problem when testing on a phone. Any idea why ? Hey Maxime, I believe you just need to add new line with "DOMError" to JSHint config here in  to make linter aware about this type.  https://github.com/mozilla-b2g/gaia/blob/1dd8ad8f96988afebc9691e1b818fa37aa32c790/apps/sms/js/compose.js#L10
Hey Maxime, I took the liberty of adding "DOMError" string to the JSHint annotation that I mentioned in comment 14 + rebased your PR on the latest master to get the latest Threeherder results. Obviously you're preserved as patch\commit author, I've just done some bureaucratic stuff for you :) Thanks for the great work!
Attachment #8525427 - Flags: review+
Treeherder is green now! Master: https://github.com/mozilla-b2g/gaia/commit/901cdf2018a3be97d696d5665c9b93669b9f5eca
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Okay thanks a lot ^^
You need to log in before you can comment on or make changes to this bug.