mixpanel-swift: [3.0.0] Rare crash inside Track.swift
Hey team, congrats on 3.0.0! I am however still seeing an (admittedly rare) crash in prod. My automated crash reporting seems to think it’s in line 34 of Track.swift. It also seems to happen extremely early in the app boot cycle, always within the first 500ms or so.
Looking at the code, I was surprised to see that ev is still force-unwraped. IIRC I had a PR where that was changed to a crash-safe if-let implementation. @zihejia Would you mind explaining the rationale behind keeping var ev force unwrapped?
Current Code:
var ev = event
if ev == nil || ev!.isEmpty {
Logger.info(message: "mixpanel track called with empty event parameter. using 'mp_event'")
ev = "mp_event"
}
My Suggestion:
var ev = "mp_event"
if let hasEv = event {
ev = hasEv
} else {
Logger.info(message: "mixpanel track called with empty event parameter. using 'mp_event'")
}
This removes any need for force unwrapping later. Also, while I can’t prove this, I don’t trust the force unwrap in the current code’s if statement to be safe. I know that for if (A || B), A should always be checked first, but I wouldn’t put it past Apple/the swift compiler optimizations in release mode to occasionally check B before A. When B is a run-time crash if A wasn’t checked first I feel that’s just asking for trouble.
This may or may not be the cause of the rare crash, but IMO it’s worth changing either way. Thanks and have a good one!
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 30 (20 by maintainers)
Looks fixed to me! Closing this 😃
We haven’t shipped to prod in a while/since this fix went live in the mixpanel lib. We should be shipping a release within a week and I can report back then!
Update from me: no longer crashes.