zevarito / mixpanel

Simple lib to track events in Mixpanel service. It can be used in any rack based framework.
MIT License
274 stars 84 forks source link

ActionDispatch::Cookies::CookieOverflow when @mixpanel.append gets called a lot. #49

Open ghost opened 12 years ago

ghost commented 12 years ago

My code uses @mixpanel.append almost exclusively for tracking events (with persist = true), so that they always fire client-side. It works great until either too many append's get called in a row or somehow the client gets stuck and their queue never clears. This eventually leads to their cookie filling up, and then they get a ActionDispatch::Cookies::CookieOverflow from too many Mixpanel events stored in the session. Unfortunately the user can't get out of this issue unless they clear their session.

I think the queue that stores client-side events should have an option to limit it's size, since the session cookies those queue'd events get stored in have a size limit.

rschmukler commented 12 years ago

What would be the ideal behavior for this? Should it simply ignore events after the queue has exceeded the session size?

Usually a controller only has one or two events. I used to ran into this problem when I was setting special API calls too frequently (such as in a before_filter to call the APIs identify, and register on every request). The mixpanel javascript API will store those variables in the cookie and therefor only need to be called once at sign in/sign up.

I'd recommend using ActiveRecord::SessionStore if your queue is getting too big. This removes the 4KB limitation.

I'm open to ideas though.

zevarito commented 12 years ago

I am not sure, honestly I am not using that feature. I have to review the implementation because I even remember how it is now. However pull requests on this are very welcome.

ghost commented 12 years ago

You're right, @rschmukler . I think the best thing for me to do is to use ActiveRecord::SessionStore. Or perhaps I'll switch to using the server-side async API since the reason I didn't use it in the first place was a lack of server-side people API calls (I'm on 2.2.0 right now). Thanks!

I like the idea of the queue rejecting more entries when it has exceeded the session size, though, just to prevent CookieOverflow from happening. I think I'll put together a pull request and see what you think.

rschmukler commented 12 years ago

@angeltinfoil the thing that'll become difficult is that the gem would then have to check the length of the session, not just the length of the queue (as it depends on what else is in the session for how much room the queue can take). The problem with the server-side-async api is that you don't get the user's cookie so you can't get them to show up in the stream, and the register/identify calls become null and void. (Correct me if I am wrong on this, but when I last tested, which was in the 2.X.X days that's how it worked).

ghost commented 12 years ago

@rschmukler You're totally right. That's why I used 'append' for all of my events in the first place. I do see some new people APIs in Mixpanel 3.X.X so I'm going to try this out and see if this allows me to switch all of my stuff to server-side.