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)

Product:
Pancake Graveyard
Component:
Front-end
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(Not tracked)

Status:
RESOLVED FIXED

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.
Didn't really think a screenshot was necessary here, honestly. The JS error dialogs all look exactly the same :)
We also log the errors on the server.
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
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
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
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
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
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.
Ok, unblocked. Tried to reproduce with the steps above with no success, it all worked perfectly :(
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.
After the above happens, tapping on social items does not work anymore.
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
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
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.
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.
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]"
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
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.
sfoster: did you want me to take this bug (making sure I was supposed to be assigned)?
Assignee: gbrander → sfoster
Status: NEW → ASSIGNED
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.