Need to expose/report SDK failures to the user

VERIFIED FIXED in mozilla33

Status

P1
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: standard8, Assigned: aoprea)

Tracking

unspecified
mozilla33
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [p=1])

User Story

This needs to happen at both ends of the conversation. Use of the existing notification infrastructure should help.

The SDK has an Exception event that appears we need to listen for it in the ConversationModel, and then take appropriate action:

http://tokbox.com/opentok/libraries/client/js/reference/ExceptionEvent.html

Attachments

(2 attachments, 4 obsolete attachments)

I've had a couple of failures from the SDK tonight.

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://anvil.opentok.com/session/1_MX40NDgzNTg5Mn5-VHVlIEp1biAxMCAxMjoyNDoxMyBQRFQgMjAxNH4wLjcwMzUxNjl-UH4?extended=true. This can be fixed by moving the resource to the same domain or enabling CORS
"getJSON said error:" error { target: XMLHttpRequest, lengthComputable: false, loaded: 0, total: 0, isTrusted: true, eventPhase: 0, bubbles: false, cancelable: false, defaultPrevented: false, timeStamp: 1402428253880141, originalTarget: XMLHttpRequest } opentok.min.js:348
"Could not connect to the OpenTok API Server." opentok.min.js:50
"OT.exception :: title: Connect Failed (1006) msg: Could not connect to the OpenTok API Server." opentok.min.js:50 

The other one was an invalid token reported from the sdk on the console.

Neither of these were reported to the user - the UI just stayed the same.

Setting as P1 as we should fix this sooner rather than later.

Comment 1

5 years ago
https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#link-prompt
see console for details...
Whiteboard: p=1
User Story: (updated)
Whiteboard: p=1 → [p=1]
Assignee: nobody → aoprea
Created attachment 8444611 [details] [diff] [review]
Handle SDK failures when calling connect
Created attachment 8444615 [details] [diff] [review]
Handle SDK failures when calling connect
Attachment #8444611 - Attachment is obsolete: true
Attachment #8444615 - Flags: review?(dmose)
Marking as blocking both the standalone link clicker UI and client UI "Call failed" notifications as I assume this occurs during call set-up both on the client UI and standalone link clicker UI.

Can the user do anything about it apart from trying to call again? If not then we should use the UI already planned in 1000186 and 1000240
Blocks: 1000186, 1000240
Yes this occurs during setup (the .connect() call) and I agree that more improvements should be made to the 'call failed' experience, but that should be left for separate bugs. Right now having this patch in would catch several error issues that were so far overlooked (both in test cases and production)

This is the way it looks now [1] it would provide some form of feedback to the user explaining in plain terms that something went wrong. Also a dev might be interested in having a closer look at the error message that gets logged in the console.

[1] http://i.imgur.com/XyjAFFn.png
Comment on attachment 8444615 [details] [diff] [review]
Handle SDK failures when calling connect

r=dmose for the most current version of the patch, which has all the issues addressed.
Attachment #8444615 - Flags: review?(dmose) → review+
Created attachment 8445509 [details] [diff] [review]
Handle SDK failures when calling connect
Attachment #8445509 - Flags: review?(dmose)
Attachment #8444615 - Attachment is obsolete: true
Created attachment 8445510 [details]
Screen Shot 2014-06-24 at 2.45.43 PM.png
Attachment #8445510 - Flags: ui-review?(dhenein)
Attachment #8445509 - Flags: review?(dmose) → review+
(In reply to Romain Testard [:RT] from comment #4)
> 
> Can the user do anything about it apart from trying to call again? If not
> then we should use the UI already planned in 1000186 and 1000240

While you're right that we want the better UI, there's a lot to like about getting this into the hands of dogfooders ASAP, before that UI is implemented.  I'd propose we land this with the UI as currently up-for-review, and then do the better UI in those bugs.
I agree with Dan.  We want better UI, but this is much better than the current state (no message at all).  I like also that it points dogfooders to check the console.
Comment on attachment 8445510 [details]
Screen Shot 2014-06-24 at 2.45.43 PM.png

Great for now, lets follow up UI in another bug as discussed.
Attachment #8445510 - Flags: ui-review?(dhenein) → ui-review+
Created attachment 8445562 [details] [diff] [review]
Handle SDK failures when calling connect, r=dmose, ui-r=darrin
Attachment #8445509 - Attachment is obsolete: true
(In reply to Dan Mosedale (:dmose - needinfo? me for responses) from comment #9)
> (In reply to Romain Testard [:RT] from comment #4)
> > 
> > Can the user do anything about it apart from trying to call again? If not
> > then we should use the UI already planned in 1000186 and 1000240
> 
> While you're right that we want the better UI, there's a lot to like about
> getting this into the hands of dogfooders ASAP, before that UI is
> implemented.  I'd propose we land this with the UI as currently
> up-for-review, and then do the better UI in those bugs.

Yes totally agree, let's go with this for now.
I captured the need to report failure to connect with the other peer at call set-up related to SDK issues on 1000186 (Desktop client call failed notification bucket) and 1000240 (standalone UI call failed notification bucket) so we can address this with a better UI later.
sorry had to backout for marionette test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=42432715&tree=Mozilla-Inbound
Backed out due to test failures:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6cbfe5dd1a1d

03:16:46     INFO -  AssertionError: 1 failure(s) encountered:
03:16:46     INFO -  should call connect
03:16:46     INFO -  expected connect to be called with exact arguments typeOf("string"), typeOf("string"), undefined
03:16:46     INFO -      connect(apiKey, sessionToken, function () {})
03:16:46     INFO -  fail@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/sinon-1.9.0.js:4687:25
03:16:46     INFO -  failAssertion@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/sinon-1.9.0.js:4648:9
03:16:46     INFO -  mirrorPropAsAssertion/assert[name]@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/sinon-1.9.0.js:4671:17
03:16:46     INFO -  @http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/models_test.js:174:1
03:16:46     INFO -  Runnable.prototype.run@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4336:24
03:16:46     INFO -  Runner.prototype.runTest@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4724:5
03:16:46     INFO -  next/<@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4802:7
03:16:46     INFO -  next@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4649:1
03:16:46     INFO -  next/<@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4659:7
03:16:46     INFO -  next@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4597:1
03:16:46     INFO -  next/<@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4621:7
03:16:46     INFO -  Runnable.prototype.run@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4338:5
03:16:46     INFO -  next@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4609:5
03:16:46     INFO -  Runner.prototype.hook/<@http://localhost:49677/build/tests/marionette/tests/browser/components/loop/test/shared/vendor/mocha-1.17.1.js:4626:5
Created attachment 8445987 [details] [diff] [review]
Handle SDK failures when calling connect
Attachment #8445562 - Attachment is obsolete: true
Attachment #8445987 - Flags: ui-review+
Attachment #8445987 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5465fbff0676
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Does this need manual testing or is it sufficiently covered by automation?
Whiteboard: [p=1] → [p=1][qa?]
I missed this bug report in my triage but I'm calling this verified fixed based on prior testing. Sorry for missing this earlier.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
QA Contact: anthony.s.hughes
Whiteboard: [p=1][qa?] → [p=1]

Updated

4 years ago
See Also: → bug 1169456
You need to log in before you can comment on or make changes to this bug.