Middleware causing app to blow up

[chrisvariety]

Hi,

First of all: Thanks for this gem. It is super great. :star2:

We recently got a low-level exception reported from this line: https://github.com/zevarito/mixpanel/blob/master/lib/mixpanel/middleware.rb#L120

The exception was:

NoMethodError: undefined method `empty?' for nil:NilClass

Here's the full stacktrace: https://gist.github.com/speedmanly/f24e7bbcda880dbf9bbd

We're on a slightly old version of the gem, but this line still appears in master.

It seems you can get into a situation where @env['rack.session'] has the key mixpanel_events, but it is nil.

I think rewriting that line to something like this would be the solution:

return [] if !(@env['rack.session']).has_key?('mixpanel_events') || Array.wrap(@env['rack.session']['mixpanel_events']).empty?

Array.wrap is provided by activesupport though, to avoid that dependency I think you might want to break it out into two lines, e.g.

mixpanel_events = @env['rack.session']['mixpanel_events'] || []
return [] if mixpanel_events.empty?

But there are lots of other ways you can accomplish the same thing.

Wish I could provide a reproduction test case, but this was just spat out of our exception notifier.

Thanks!

:rainbow:

[zevarito]

Hi,

Sorry for the late response.

A test case will be really useful since I didn't hear something like this were happening before.

In my opinion adding an [] fallback will make hard to find bugs in the future, accordingly to the source code mixpanel_events should be never set to nil, and if its so, something bad is happening.

I will close this for now, fell free to reopen it if needed.