Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Harden telemetry event dispatch #8767

Merged
merged 2 commits into from
Apr 20, 2017
Merged

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Apr 19, 2017

Closes #8650, this PR introduces a wrapper implementation of MapboxEvent. This is needed to ignore events when the Map isn't ready to handle them (when we don't have a camera position to get the zoom level from). A use-case where this can occur is running monkeyrunner, note that I haven't been able to reproduce with normal behavior.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Apr 19, 2017
@tobrun tobrun added this to the android-v5.1.0 milestone Apr 19, 2017
@tobrun tobrun self-assigned this Apr 19, 2017
@tobrun tobrun requested a review from zugaldia April 19, 2017 09:58
@tobrun tobrun merged commit 86433df into master Apr 20, 2017
@tobrun tobrun deleted the 8650-harden-telem-event-dispatch branch April 20, 2017 14:18
@tobrun tobrun mentioned this pull request May 2, 2017
12 tasks
@tobrun tobrun mentioned this pull request Jun 9, 2017
12 tasks
@tobrun tobrun mentioned this pull request Jun 21, 2017
11 tasks
@tobrun tobrun mentioned this pull request Jun 30, 2017
16 tasks
Copy link

@Larnoo Larnoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+    try {
 +      return MapboxEvent.buildMapClickEvent(location, gestureId, transform.getZoom());
 +    } catch (NullPointerException exception) {
 +      // Map/Transform is not ready yet #8650
 +      // returning null is valid, event is ignored.
 +      return null;
 +    }

why catch NPE? If catch NPE, other object is null will hidden the problem.
catching Exception is costly.

I think should do as follow.

if(transform==null){
//the transform is null; can't do anything. or print log.
  return null;
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants