Closed
Bug 1031993
Opened 10 years ago
Closed 10 years ago
NFC - JavaScript Error: "Unable to create NFCPeer object"
Categories
(Firefox OS Graveyard :: NFC, defect)
Tracking
(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: allstars.chh, Assigned: arno)
References
Details
Attachments
(3 files, 2 obsolete files)
STR: 1. Turn off BT in receiver side. 2. On sender side, using Shrinking-UI to do the file transfer. 3. Take two devices away. Expected Result: If the Handover-Request/Select is interrupted (the other phone is taken away), we should pop up the error to notify the user. Actual Result: When the Handover is interrupted, no error notification will be shown. The receiver side tries to respond Handover Select, but the sender side is already taken away, so when calling nfc.getNFCPeer(session), it will throw "JavaScript Error: "Unable to create NFCPeer object", but the error is not shown so user cannot know the handover procedure is actually failed. See the log from https://bugzilla.mozilla.org/show_bug.cgi?id=1029303#c4
Reporter | ||
Comment 2•10 years ago
|
||
Reporter | ||
Comment 3•10 years ago
|
||
In Comment 2 I uploaded the log captured from the sender with debug enabled from Gecko NFC part, nfc_manager.js, and nfc_handover_manager.js. STR: Sender device disables BT and enables NFC. Tapping another device After swiping, take the two phones away immediately. From the log, line 6556 E/GeckoConsole( 290): [JavaScript Error: "Unable to create NFCPeer object, Reason: Bad SessionToken {58e7f7a8-beba-4dac-af9f-719850b3101c}" {file: "jar:file:///system/b2g/omni.ja!/components/nsNfc.js" line: 200}] This seems normal since after line 6383 the session is already invalid. However now disable the BT again, and tap the two phones together. New session starts from line 6687, with session id {9b8f6924-291e-46c2-adab-506d5283ace6} Then do the swipe the share the data, in line 8065, NFCPeer.sendFile is called, then the message is broadcasted to nfc_handover_manager.js again, and it will try to enable BT again. line 9054 the BT is enabled, now nfc_handover_manager will call initiateFileTransfer. However the arguments for this function, is WRONG. From log line 9096, we will see it tries to getNFCPeer with old session info. I/Gecko ( 290): -*- Nfc: Received 'NFC:SetSessionToken' message from content process I/Gecko ( 290): -*- Nfc: Received invalid Session Token: {58e7f7a8-beba-4dac-af9f-719850b3101c} - Do not register this target E/GeckoConsole( 290): [JavaScript Error: "Unable to create NFCPeer object, Reason: Bad SessionToken {58e7f7a8-beba-4dac-af9f-719850b3101c}" {file: "jar:file:///system/b2g/omni.ja!/components/nsNfc.js" line: 200}] E/GeckoConsole( 290): [JavaScript Error: "NS_ERROR_UNEXPECTED: " {file: "app://system.gaiamobile.org/js/nfc_handover_manager.js" line: 249}] Note the session now should be {9b8f6924-291e-46c2-adab-506d5283ace6}, instead of {58e7f7a8-beba-4dac-af9f-719850b3101c}. I think there's a bug in maintaining the 'actionQueue', if one action throws error, it's never deleted, so when the actionQueue is popping up again, always the oldest one will be executed, and throwing error again. Nominate this to 2.0+ for this also happens on 2.0 branch.
blocking-b2g: --- → 2.0?
Reporter | ||
Comment 4•10 years ago
|
||
I think there should be two problems here, 1. We should notify the user if the other device is already away, otherwise user will keep waiting however nothing will happen, as mentioned Comment 0. 2. Fix the actionQueue bug mentioned in Comment 3
Comment 5•10 years ago
|
||
triage: 2.0+ because this breaks user experience. user would get confused what goes wrong.
blocking-b2g: 2.0? → 2.0+
(In reply to Yoshi Huang[:allstars.chh] from comment #4) > I think there should be two problems here, > 1. We should notify the user if the other device is already away, otherwise > user will keep waiting however nothing will happen, as mentioned Comment 0. I left a NI for Juwei in bug 894320 to clarify what the UI should look like. > 2. Fix the actionQueue bug mentioned in Comment 3 I will look into this.
(In reply to Yoshi Huang[:allstars.chh] from comment #4) > 2. Fix the actionQueue bug mentioned in Comment 3 The problem is not with actionQueue but sendFileQueue. What happened is that NfcHandoverManager uses getNFCPeer(). If the peer already went away (e.g., because you immediately remove the other device) it will throw an exception. Since there is no try-block to catch that exception, no proper cleanup happens and for that reason BT is not turned off again and there is an orphaned request in sendFileQueue. I will fix it and the notification but since the patch depends on Bug 998175 I will wait for the latter to land before submitting a patch for review.
Comment 8•10 years ago
|
||
(In reply to arno from comment #6) > (In reply to Yoshi Huang[:allstars.chh] from comment #4) > > I think there should be two problems here, > > 1. We should notify the user if the other device is already away, otherwise > > user will keep waiting however nothing will happen, as mentioned Comment 0. > > I left a NI for Juwei in bug 894320 to clarify what the UI should look like. > Reply by Juwei is here: https://bugzilla.mozilla.org/show_bug.cgi?id=894320#c35 > > 2. Fix the actionQueue bug mentioned in Comment 3 > > I will look into this.
Comment 9•10 years ago
|
||
Regarding schedule, 2.0 blocker should be fixed before end of last sprint.
Target Milestone: --- → 2.0 S6 (18july)
Updated•10 years ago
|
Blocks: b2g-NFC-2.0
Reporter | ||
Comment 10•10 years ago
|
||
(In reply to Yoshi Huang[:allstars.chh] from comment #4) > I think there should be two problems here, > 1. We should notify the user if the other device is already away, otherwise > user will keep waiting however nothing will happen, as mentioned Comment 0. > 2. Fix the actionQueue bug mentioned in Comment 3 Comment 4 is made before this bug is nominated to 2.0+, given that this is a 2.0+ bug now, we could fix the 2nd problem (actionQueue) first, and file another bug for the first.
Assignee | ||
Comment 11•10 years ago
|
||
Yoshi: I was already sitting on a patch that addresses both issues you have raised. Let me know what you think.
Attachment #8453083 -
Flags: review?(allstars.chh)
Reporter | ||
Comment 12•10 years ago
|
||
Comment on attachment 8453083 [details]
Fix getNFCPeer() bug and show notification when transfer fails
It's in gaia part so forward r? to Alive.
Attachment #8453083 -
Flags: review?(allstars.chh) → review?(alive)
Comment 13•10 years ago
|
||
Comment on attachment 8453083 [details] Fix getNFCPeer() bug and show notification when transfer fails See github, and ask gweng@mozilla.com to review again. Thank you.
Attachment #8453083 -
Flags: review?(alive)
Assignee | ||
Comment 14•10 years ago
|
||
Fixed Alive's github comments.
Attachment #8453083 -
Attachment is obsolete: true
Attachment #8453856 -
Flags: review?(gweng)
Comment 15•10 years ago
|
||
Comment on attachment 8453856 [details]
Fix getNFCPeer() bug and show notification when transfer fails
It's without what Alive's addressed. Unfortunately, the linter error occurs. Please fix it and set the flag again.
Attachment #8453856 -
Flags: review?(gweng)
Assignee | ||
Comment 16•10 years ago
|
||
I just double checked and I did address all of Alive's comments. Can you please tell me which one I did not fix?
Flags: needinfo?(gweng)
Comment 17•10 years ago
|
||
Oh, sorry, typo. What I mean is you fixed all that Alive commented, but it failed to pass linter check: https://tbpl.mozilla.org/?rev=16dd81f7c1c345a567783582c0e7701eaf9c7f87&tree=Gaia-Try and Travis linter check: https://travis-ci.org/mozilla-b2g/gaia/builds/29617692
Flags: needinfo?(gweng)
Assignee | ||
Comment 18•10 years ago
|
||
Make lint happy.
Attachment #8453856 -
Attachment is obsolete: true
Attachment #8455305 -
Flags: review?(gweng)
Comment 19•10 years ago
|
||
Comment on attachment 8455305 [details]
Fix getNFCPeer() bug and show notification when transfer fails
Okay, it passed all tests and code is okay now, thanks.
Attachment #8455305 -
Flags: review?(gweng) → review+
Keywords: checkin-needed
Reporter | ||
Comment 20•10 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/c5167d0b1964fb055ad0b985b281f76b9e305219
Keywords: checkin-needed
Reporter | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 21•10 years ago
|
||
just a reminder, besides patch for master we'll need the patch for 2.0 as this is 2.0+.
Flags: needinfo?(arno)
Comment 23•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/391258012d09cc1dde9a6f3f9a2078739043d0d6
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•