Skip to content

Conversation

@Kota-Jagadeesh
Copy link
Collaborator

@Kota-Jagadeesh Kota-Jagadeesh commented Dec 9, 2025

Description

Fixes #6469

What changes did you make and why?

This pull request addressses the critical application crash (NullPointerException) that was triggered when users attempted an action requiring location dataa (like taking a picture from the "Nearby" tab) while the device was offline or had a severely flaky internet connection.

The root cause isthat the NearbyParentFragmentPresenter attempted to fetch map boundary coordinates (screenBottomLeft and screenTopRight) from the map view. When offline, these coordinates areoften uninitialized and returned null. This null value was then passed down to the Java data source method, leading to a crash when code attempted to call .getLatitude() on the null object.

The solution ensures the robust handling of the offline/uninitialized state:

  1. NearbyParentFragmentPresenter.kt: Explicit null checks aree added for screenBottomLeft and screenTopRight within the handleMapScrolled offline logic. If the map boundaries are null, the local place search job is now gracefully exited, preventing the null values from ever reaching the data layer.
  2. PlacesLocalDataSource.java: A defensive null check is implemented at the start of the fetchPlaces method. If either map boundary input is null, it now returns an empty list immediately instead of risking a NullPointerException on the map boundary processing logicc.

Tests performed (required)

Tested ProDebug on Redmi Note 13 Pro 5g with API level 35.

Screenshots (for UI changes only)

N/A (This is a code stability fix, not a UI change)

@nicolas-raoul
Copy link
Member

Note: I am getting this crash on app start frequently, but this happens in other branches too:

APP_VERSION_NAME=6.2.0-debug-Kota-Jagadeesh-fix/nearby-offline-crash-6469
ANDROID_VERSION=16
PHONE_MODEL=Pixel 10 Pro Fold
STACK_TRACE=java.lang.NullPointerException
at fr.free.nrw.commons.media.MediaDetailPagerFragment.onSaveInstanceState(MediaDetailPagerFragment.kt:128)
at androidx.fragment.app.Fragment.performSaveInstanceState(Fragment.java:3151)
at androidx.fragment.app.FragmentStateManager.saveBasicState(FragmentStateManager.java:683)
at androidx.fragment.app.FragmentStateManager.saveState(FragmentStateManager.java:649)
at androidx.fragment.app.FragmentStore.saveActiveFragments(FragmentStore.java:177)
at androidx.fragment.app.FragmentManager.saveAllState(FragmentManager.java:2655)
at androidx.fragment.app.Fragment.performSaveInstanceState(Fragment.java:3153)
at androidx.fragment.app.FragmentStateManager.saveBasicState(FragmentStateManager.java:683)
at androidx.fragment.app.FragmentStateManager.saveState(FragmentStateManager.java:649)
at androidx.fragment.app.FragmentStore.saveActiveFragments(FragmentStore.java:177)
at androidx.fragment.app.FragmentManager.saveAllState(FragmentManager.java:2655)
at androidx.fragment.app.FragmentController.saveAllState(FragmentController.java:152)
at androidx.fragment.app.FragmentActivity$1.saveState(FragmentActivity.java:133)
at androidx.savedstate.SavedStateRegistry.performSave(SavedStateRegistry.kt:247)
at androidx.savedstate.SavedStateRegistryController.performSave(SavedStateRegistryController.kt:81)
at androidx.activity.ComponentActivity.onSaveInstanceState(ComponentActivity.kt:345)
at fr.free.nrw.commons.contributions.MainActivity.onSaveInstanceState(MainActivity.kt:348)
at android.app.Activity.performSaveInstanceState(Activity.java:2539)
at android.app.Instrumentation.callActivityOnSaveInstanceState(Instrumentation.java:1769)
at android.app.ActivityThread.callActivityOnSaveInstanceState(ActivityThread.java:6900)
at android.app.ActivityThread.callActivityOnStop(ActivityThread.java:6298)
at android.app.ActivityThread.performStopActivityInner(ActivityThread.java:6265)
at android.app.ActivityThread.handleStopActivity(ActivityThread.java:6328)
at android.app.servertransaction.StopActivityItem.execute(StopActivityItem.java:43)
at android.app.servertransaction.ActivityTransactionItem.execute(ActivityTransactionItem.java:63)
at android.app.servertransaction.TransactionExecutor.executeLifecycleItem(TransactionExecutor.java:169)
at android.app.servertransaction.TransactionExecutor.executeTransactionItems(TransactionExecutor.java:101)
at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:80)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2961)
at android.os.Handler.dispatchMessage(Handler.java:132)
at android.os.Looper.dispatchMessage(Looper.java:333)
at android.os.Looper.loopOnce(Looper.java:263)
at android.os.Looper.loop(Looper.java:367)
at android.app.ActivityThread.main(ActivityThread.java:9287)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:566)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:929)

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

I can reproduce the crash in main but not on this branch, great! :-)

About the code:

Boundaries being null sounds like there was an issue higher in the callstack. Would you mind trying to identify what led to boundaries being null? Probably that's because a network call failed somewhere? If yes, better exit gracefully there, rather than down the stream.

@Kota-Jagadeesh
Copy link
Collaborator Author

Boundaries being null sounds like there was an issue higher in the callstack. Would you mind trying to identify what led to boundaries being null? Probably that's because a network call failed somewhere? If yes, better exit gracefully there, rather than down the stream.

@nicolas-raoul Thanks for verifying the fix! I agree that tracing the null boundaries upstream is the more robust approach.

I've investigated the map boundaries further by inspecting the NearbyParentFragment.kt.

The reason they appeared 'null' (or uninitialized) was twofold:

  1. Map Initialization Failure: In an offline scenario, the underlying Map SDK sometimes fails to fully initialize, leaving the projection logic (which calculates the screen corners) either invalid or returning the 'uninitialized' GeoPoint (0, 0).
  2. Logic Flow: In the Presenter's offline path (handleMapScrolled):
    • It retrieves the boundaries via nearbyParentFragmentView.screenBottomLeft (and screenTopRight).
    • This calls the Fragment getters, which were previously implemented as returning a non-nullable LatLng (the coordinate object used down the line in PlacesLocalDataSource.java).

The most logical upstream point to handle the failure of the map to initialize is right before we consume these properties in the handleMapScrolled function. Mmy the current fix explicitly checks for the null map boundary values before calling the data layer (placesRepository.fetchPlaces).

This ensures that the application flow is immediately stopped and doesn't proceed with a search if the necessary map dependencies (the corner coordinates) haven't been successfully initialized by the View layer, which is the earliest point of control we have over those specific variables.

I believe the neww commit addressess the root of the control flow issue by validating map readiness before continuing the network-independent search logic. llet me know if you would prefer to see error handling added directly within the Fragment's getScreenTopRight/getScreenBottomLeft methods instead.

@nicolas-raoul
Copy link
Member

(tested: bug does not reproduce with these commits either)

When offline, these coordinates are often uninitialized and returned null

Nearby does not crash when I go (while offline) from home activity to Nearby activity.

Upon choosing a picture, ideally the Nearby activity should not be used any further. Once a picture has been chosen, we just show the upload wizard, and do not need to call fetchPlaces, right?

@Kota-Jagadeesh
Copy link
Collaborator Author

Kota-Jagadeesh commented Dec 21, 2025

@nicolas-raoul Sorry about this! I’m in the middle of my end-sem exams right now, so I couldn’t find time to work on the issue. I’ll continue to work on thiss after the 27th 🙂

@nicolas-raoul
Copy link
Member

Good luck with your exams! 🙂

@Kota-Jagadeesh
Copy link
Collaborator Author

Hi @nicolas-raoul, i have updated the fix to address the root cause as suggested. instead of catching nulls at the data layer, i implemented a 'flow lock' using an isInternalUploadInProgress flag in the NearbyController. This flag is set by the ContributionController the moment an image is selected and checked by the NearbyParentFragmentPresenter at the beginning of updateMapAndList. this ensures that returning from the camera gracefully exits the refresh logic before any map coordinates are requested, preventing unnecessary calls during the transition to the upload wizard.

@github-actions
Copy link

github-actions bot commented Jan 2, 2026

✅ Generated APK variants!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: crash when attempting to take picture on nearby without Internet

2 participants