fix: move session-specific logic from ContextPlugin to avoid race conditions #86
Conversation
Micro-Learning Topic: Race condition (Detected by phrase)Matched on "race condition"A race condition is a flaw that produces an unexpected result when the timing of actions impact other actions. Try a challenge in Secure Code Warrior |
falconandy
force-pushed
the
DXOC-261-sessions-weird-behavior
branch
from
aa11870 to
5973986
Compare
| sendSessionEvent(END_SESSION_EVENT) | ||
| val options = EventOptions() | ||
| options.timestamp = if (lastEventTime > 0) lastEventTime + 1 else null | ||
| sendSessionEvent(END_SESSION_EVENT, options) |
There was a problem hiding this comment.
lastEventTime + 1 is for?
There was a problem hiding this comment.
I am not sure how events are ordered in UI. I added 1 millisecond to ensure the session_end event is last. Maybe it is unnecessary.
There was a problem hiding this comment.
I see, I guess it won't hurt much.
There was a problem hiding this comment.
Changed to lastEventTime
| override fun processEvent(event: BaseEvent) { | ||
| val eventTimestamp = event.timestamp ?: System.currentTimeMillis() | ||
| event.timestamp = eventTimestamp | ||
| if (lastEventTime < eventTimestamp) { |
There was a problem hiding this comment.
this will break the logic for isWithinMinTimeBetweenSessions if the event falls into the later startNewSessionIfNeeded, maybe we should move this to the bottom
There was a problem hiding this comment.
Removed that block - it looks like refreshSessionTime already does it
if (lastEventTime < eventTimestamp) {
lastEventTime = eventTimestamp
}
There was a problem hiding this comment.
@qingzhuozhen
I am thinking about tests
|
@qingzhuozhen |
| sendSessionEvent(END_SESSION_EVENT) | ||
| val options = EventOptions() | ||
| options.timestamp = if (lastEventTime > 0) lastEventTime + 1 else null | ||
| sendSessionEvent(END_SESSION_EVENT, options) |
There was a problem hiding this comment.
I see, I guess it won't hurt much.
| val sessionEndEvent = BaseEvent() | ||
| sessionEndEvent.eventType = END_SESSION_EVENT | ||
| sessionEndEvent.timestamp = if (lastEventTime > 0) lastEventTime else null | ||
| sessionEndEvent.sessionId = sessionId |
There was a problem hiding this comment.
if we set the eventId here, I guess it could also be fine?
| if (configuration.optOut) { | ||
| logger.info("Skip event for opt out config.") | ||
| return | ||
| } | ||
|
|
||
| val sessionEvents = processEvent(event) |
There was a problem hiding this comment.
Maybe not call it sessionEvents? just events are fine. This way seems to be coupling the logic in core and android package.
## [1.4.5](v1.4.4...v1.4.5) (2022-10-19) ### Bug Fixes * move session-specific logic from ContextPlugin to avoid race conditions ([#86](#86)) ([311c230](311c230))
|
🎉 This PR is included in version 1.4.5 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary
Session-specific logic moved from ContextPlugin to avoid race conditions
Checklist