Closed
Bug 769418
Opened 12 years ago
Closed 12 years ago
Getting a bunch of JS errors when clicking social items
Categories
(Pancake Graveyard :: Front-end, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
M3
People
(Reporter: deb, Assigned: sfoster)
Details
Something with social items is weird -- I keep getting a JS error dialogs when clicking social items, which causes the drawer state to get really confused. I'm trying to replicate with search results, but only social has caused the issue so far.
I'm using my drichardson@mozilla.com account if the logs from this afternoon would be helpful there.
To note:
You can take screenshots on the iPad by holding the power and the home button at the same time. It will store them in your pictures, just connect them to your Mac and it will launch iPhoto and you can import those pictures and attach them to the bug.
Reporter | ||
Comment 2•12 years ago
|
||
Didn't really think a screenshot was necessary here, honestly. The JS error dialogs all look exactly the same :)
Comment 3•12 years ago
|
||
We also log the errors on the server.
Reporter | ||
Comment 4•12 years ago
|
||
I reproduced this with my deb@mozilla.com account as well, but it's currently less consistent there right now.
IRC notes from oyiptong:
5:04 PM oyiptong: same error: http://play.fxhome.mozillalabs.com:9000/default/group/23/
5:04 PM oyiptong: that should probably help sfoster hunt down the bug
5:04 PM dria: cool
5:04 PM oyiptong: btw, sfoster , the exception title could be improved =)
5:04 PM oyiptong that's the error: Error: {"message":"KeysMissingError: Results array not found in PlaceCollection fetch response data: d","name":"Error","details":""}
Assignee: nobody → sfoster
Assignee | ||
Comment 5•12 years ago
|
||
I can't reproduce this on dev (play). I'm trying clicking twitter and facebook items in the social feed and all is well. From the looks of that error it seems the FE was getting bad data(!!!) from lattice - if its complaining about the results array not found.
So, the error was caught, but I'm not sure how we want to handle it on the FE. Throwing the exception up in the alert is just a while-we're-testing measure, we need to come up with user-friendly messaging.
Yes I'll look at fixing that exception title meantime.
Assignee: sfoster → oyiptong
Assignee | ||
Comment 6•12 years ago
|
||
I did push a fix to an issue that gives a default place_title to places the user navigates to; pages without title showed up as an empty string in the /link/ request we send to lattice, which resulted in an error. I don't think that's this issue (and its not pushed to dev yet).
https://bitbucket.org/mozillapancake/pancake/changeset/89e4ae029798
Comment 7•12 years ago
|
||
Here is a HTTP trace of the last few calls to Lattice that happen. The social item clicked on was the Design Milk one, which is the one before the POST to the exception logger:
http://st3fan.pastebin.mozilla.org/1683948
Comment 8•12 years ago
|
||
I think I found a way to reproduce this:
1) Wipe your account to be sure
2) Connect to twitter
3) Click on a social item
4) Go back to the social feed
5) Click on another social item BY THE SAME AUTHOR
This will result in the error happening
It might take some effort to find two items from the same author. Easiest is to setup a twitter account and be friends with just one or two high volume accounts like HackersNews or BlogTO
Assignee | ||
Comment 9•12 years ago
|
||
I'm currently blocked on this as when I try to run ./scripts/social.sh to start the social app, I get:
[I 120703 11:24:11 pancake-social-server:23] Starting pancake-social on 127.0.0.1:4324
Traceback (most recent call last):
File "/Users/sfoster/dev/moz/pancake/pancake-social/scripts/pancake-social-server", line 25, in <module>
application = SocialApplication()
File "/Users/sfoster/dev/moz/pancake/pancake-social/pancake/social/main.py", line 296, in __init__
self.metlog = metlog.holder.get_client(**self.metlog_settings)
TypeError: get_client() argument after ** must be a mapping, not NoneType
I've double-checked my web.json and social.json in ~/.pancake, but they look good. I've also run ./scripts/setup-social.sh again.
I can workaround by doing some proxy shenanigans if necessary, but meantime I'm going to try to improve the error handling to get us more to work with for this and similar issues.
Assignee | ||
Comment 10•12 years ago
|
||
Ok, unblocked. Tried to reproduce with the steps above with no success, it all worked perfectly :(
Comment 11•12 years ago
|
||
We don't see the JS error anymore because you added this check on friday:
https://bitbucket.org/mozillapancake/pancake/changeset/6379776b9b22
But the underlying problem is still there.
As a result of this, the drawer is also confused .. it has two items selected.
I can reproduce that with the steps from comment 8.
Comment 12•12 years ago
|
||
After the above happens, tapping on social items does not work anymore.
Assignee | ||
Comment 13•12 years ago
|
||
stack.collection doesn't exist when you create a new stack. It turns out it is only added as a side-effect of stack.addTo(stacks, { at: 0 }) in augmentStacksWithStack (drawer.app.js). So, if the stack is retrieved with stacks.get(stack.id), it never gets added at all.
In https://bitbucket.org/mozillapancake/pancake/changeset/043d582e37d3, I've added the .collection reference explicitly there. This seems to be work, but still seems like a kludge. The issue is that clicking on the social post of a friend we already have a stack for shouldn't be firing social:create in the first place. However, due to how our data is arranged, its hard to know that in main.
Maybe there's a better fix though where we look for an existing stack for this friend before creating it.
To gordon for review.
Assignee: oyiptong → gbrander
Comment 14•12 years ago
|
||
Hmm, I'm confused. stack.collection should be set by Backbone as soon as the model has been added to a collection. If we're getting the stack from the stack collection, it stands to reason it would have a collection already, right?
Also, I saw the exception appear in _movePlaceInPlacesAndAnimate (at the end of the Promise chain), but that might be the way the Promise handles exceptions that is fooling me... no helpful traceback to work with.
sfoster: can we chat about this fix?
Assignee: gbrander → sfoster
Assignee | ||
Comment 15•12 years ago
|
||
I'm confused also. Ive spent more time today poking at this, logging and re-arranging bits. I *think* that this is a symptom of the way we conflate a Promise and a Deferred in the promise.js, and how chaining works with the 2 kinds of objects. A promise should pass the same value to all its callbacks, whereas a deferred passes along the return value of the previous callback - as I understand it. (I thought I understood it but the last couple hours have made me doubt.. need to test some more)
Anyhow, yes I'm out some of this evening but will be on later on and should sit down and talk this through.
Assignee | ||
Comment 16•12 years ago
|
||
Re: https://bitbucket.org/mozillapancake/pancake/changeset/0d012dd0d04a
I spent some time breaking out the long promise chain in toStackAndPlace to allow me to log and step through easier. There was some ambiguity in there with some methods returning either a promise or a value, the only actual issue I could find was that the wait helper wasn't passing in the value to the callback. I know the docs say it should, but adding in the anon function there seems to have done the trick.
Along the way I added this custom optionally-async sequence for some of the function calls which didn't need to be chained (meaning the didn't rely on being passed the previous function's return value).
I'm not 100% on this. I'm no longer able to repro the original issue, but I've seen a couple of times where the places list in the drawer fails to open when a social post is clicked.
Comment 17•12 years ago
|
||
Hmm I'm still seeing the confused drawer. See https://moo.mx/grabs/07445728.png
Two items are selected. When I navigate to another stack and then try to open the above broken stack, nothing happens.
I see this in the iOS console:
2012-07-05 10:50:43.519 PancakeDevelopment[85177:c07] WARN - DRAWER - "Promise was rejected", {
stack: "@http://play.fxhome.mozillalabs.com:6543/js/drawer.app.js?r=20120410007:304\nnotify@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:199\nnotifyAll@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:166\nemitSuccess@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:227\nnotify@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:199\nnotifyAll@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:166\nemitError@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:232\nnotify@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:199\nnotifyAll@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:166\nemitError@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:232\nnotify@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:199\nthen@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:253\nnotify@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:199\nnotifyAll@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:166\nemitSuccess@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:227\n@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:335\nnotify@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:199\nnotifyAll@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:166\nemitSuccess@http://play.fxhome.mozillalabs.com:6543/js/lib/promise.js?r=20120410007:227\n@http://play.fxhome.mozillalabs.com:6543/js/drawer.app.js?r=20120410007:55\n[native code]"
Assignee | ||
Comment 18•12 years ago
|
||
This must be a race condition, as I'm unable to reproduce this locally, but can when running on dev/play.
In FF you get a usable stack trace, it flags up drawer.app.js _moveStack. Turns out that the stacks.indexOf(stack) returns -1 because stack !== stacks.models[0] - even though they represent the same data.
I've added a workaround for this, but the actual logical error and/or race condition must be upstream somewhere.
See: https://bitbucket.org/mozillapancake/pancake/changeset/d21df493960e
Assignee: sfoster → gbrander
Comment 19•12 years ago
|
||
I don't see the warning about the Promise in the the console anymore but I can still confuse the drawer with the original Steps To Reproduce. There is no way to recover from that except to restart the app.
Comment 20•12 years ago
|
||
sfoster: did you want me to take this bug (making sure I was supposed to be assigned)?
Assignee: gbrander → sfoster
Status: NEW → ASSIGNED
Assignee | ||
Comment 21•12 years ago
|
||
Band-aid applied in https://bitbucket.org/mozillapancake/pancake/changeset/5e60afea4b11
.. at various points the stack (StackModel instance) passed into a function !=== app.stacks.get(stack.id) (although ids and other details did match). I suspect this is related to the memoized promises in StackCollection's fetchStack, but I've not been able to figure out exactly how. However, normalizing by going back to app.stacks' model when ids match seems to fix this issue.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•