Skip to content

Commit a8ddabb

Browse files
jokerttujokerttu
authored
fix!: navigation session error handling (#502)
Improves error handling and guards for navigation session method calls. Adds NO_DESTINATIONS error for android implementation if start guidance is called without destinations set. BREAKING CHANGE: `no_destinations` error string on iOS is changed to `NO_DESTINATIONS`, matching other error code formats and Android implementation
1 parent 6e0cd58 commit a8ddabb

File tree

12 files changed

+558
-176
lines changed

12 files changed

+558
-176
lines changed

‎android/src/main/java/com/google/android/react/navsdk/JsErrors.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,7 @@ public class JsErrors {
2121
public static final String NO_MAP_ERROR_CODE = "NO_MAP_ERROR_CODE";
2222
public static final String NO_MAP_ERROR_MESSAGE =
2323
"Make sure to initialize the map view has been initialized before executing.";
24+
25+
public static final String NO_DESTINATIONS_ERROR_CODE = "NO_DESTINATIONS";
26+
public static final String NO_DESTINATIONS_ERROR_MESSAGE = "Destinations not set";
2427
}

‎android/src/main/java/com/google/android/react/navsdk/NavModule.java‎

Lines changed: 76 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,11 @@ public Map<String, Object> getConstants() {
151151
}
152152

153153
@ReactMethod
154-
private void cleanup() {
154+
public void cleanup(final Promise promise) {
155+
if (!ensureNavigatorAvailable(promise)) {
156+
return;
157+
}
158+
155159
stopUpdatingLocation();
156160
removeNavigationListeners();
157161
mWaypoints.clear();
@@ -160,10 +164,12 @@ private void cleanup() {
160164
listener.onReady(false);
161165
}
162166

167+
final Navigator navigator = mNavigator;
163168
UiThreadUtil.runOnUiThread(
164169
() -> {
165-
mNavigator.clearDestinations();
166-
mNavigator.cleanup();
170+
navigator.clearDestinations();
171+
navigator.cleanup();
172+
promise.resolve(true);
167173
});
168174
}
169175

@@ -282,6 +288,10 @@ public void onError(@NavigationApi.ErrorCode int errorCode) {
282288
public void setTurnByTurnLoggingEnabled(boolean isEnabled) {
283289
final Activity currentActivity = getReactApplicationContext().getCurrentActivity();
284290
if (currentActivity == null) return;
291+
if (mNavigator == null) {
292+
logDebugInfo(JsErrors.NO_NAVIGATOR_ERROR_MESSAGE);
293+
return;
294+
}
285295

286296
if (isEnabled) {
287297
NavForwardingManager.startNavForwarding(mNavigator, currentActivity, this);
@@ -295,6 +305,9 @@ public void setTurnByTurnLoggingEnabled(boolean isEnabled) {
295305
* navigation events occur (e.g. the driver's route changes or the destination is reached).
296306
*/
297307
private void registerNavigationListeners() {
308+
if (mNavigator == null) {
309+
return;
310+
}
298311
removeNavigationListeners();
299312

300313
mArrivalListener =
@@ -353,6 +366,9 @@ public void onRemainingTimeOrDistanceChanged() {
353366
}
354367

355368
private void removeNavigationListeners() {
369+
if (mNavigator == null) {
370+
return;
371+
}
356372
if (mArrivalListener != null) {
357373
mNavigator.removeArrivalListener(mArrivalListener);
358374
}
@@ -417,19 +433,20 @@ private void createWaypoint(Map map) {
417433
public void setDestination(
418434
ReadableMap waypoint,
419435
@Nullable ReadableMap routingOptions,
420-
@Nullable ReadableMap displayOptions) {
436+
@Nullable ReadableMap displayOptions,
437+
final Promise promise) {
421438
WritableArray array = new WritableNativeArray();
422439
array.pushMap(waypoint);
423-
setDestinations(array, routingOptions, displayOptions);
440+
setDestinations(array, routingOptions, displayOptions, promise);
424441
}
425442

426443
@ReactMethod
427444
public void setDestinations(
428445
ReadableArray waypoints,
429446
@Nullable ReadableMap routingOptions,
430-
@Nullable ReadableMap displayOptions) {
431-
if (mNavigator == null) {
432-
// TODO: HANDLE THIS
447+
@Nullable ReadableMap displayOptions,
448+
final Promise promise) {
449+
if (!ensureNavigatorAvailable(promise)) {
433450
return;
434451
}
435452

@@ -462,42 +479,63 @@ public void setDestinations(
462479
if (pendingRoute != null) {
463480
// Set an action to perform when a route is determined to the destination
464481
pendingRoute.setOnResultListener(
465-
code -> sendCommandToReactNative("onRouteStatusResult", code.toString()));
482+
code -> {
483+
sendCommandToReactNative("onRouteStatusResult", code.toString());
484+
promise.resolve(true);
485+
});
486+
} else {
487+
promise.resolve(true);
466488
}
467489
}
468490

469491
@ReactMethod
470-
public void clearDestinations() {
471-
if (mNavigator != null) {
472-
mWaypoints.clear(); // reset waypoints
473-
mNavigator.clearDestinations();
492+
public void clearDestinations(final Promise promise) {
493+
if (!ensureNavigatorAvailable(promise)) {
494+
return;
474495
}
496+
mWaypoints.clear(); // reset waypoints
497+
mNavigator.clearDestinations();
498+
promise.resolve(true);
475499
}
476500

477501
@ReactMethod
478-
public void continueToNextDestination() {
479-
if (mNavigator != null) {
480-
mNavigator.continueToNextDestination();
502+
public void continueToNextDestination(final Promise promise) {
503+
if (!ensureNavigatorAvailable(promise)) {
504+
return;
481505
}
506+
mNavigator.continueToNextDestination();
507+
promise.resolve(true);
482508
}
483509

484510
@ReactMethod
485-
public void startGuidance() {
511+
public void startGuidance(final Promise promise) {
512+
if (!ensureNavigatorAvailable(promise)) {
513+
return;
514+
}
486515
if (mWaypoints.isEmpty()) {
516+
promise.reject(JsErrors.NO_DESTINATIONS_ERROR_CODE, JsErrors.NO_DESTINATIONS_ERROR_MESSAGE);
487517
return;
488518
}
489519

490520
mNavigator.startGuidance();
491521
sendCommandToReactNative("onStartGuidance", null);
522+
promise.resolve(true);
492523
}
493524

494525
@ReactMethod
495-
public void stopGuidance() {
526+
public void stopGuidance(final Promise promise) {
527+
if (!ensureNavigatorAvailable(promise)) {
528+
return;
529+
}
496530
mNavigator.stopGuidance();
531+
promise.resolve(true);
497532
}
498533

499534
@ReactMethod
500535
public void simulateLocationsAlongExistingRoute(float speedMultiplier) {
536+
if (mNavigator == null) {
537+
return;
538+
}
501539
if (mWaypoints.isEmpty()) {
502540
return;
503541
}
@@ -510,16 +548,25 @@ public void simulateLocationsAlongExistingRoute(float speedMultiplier) {
510548

511549
@ReactMethod
512550
public void stopLocationSimulation() {
551+
if (mNavigator == null) {
552+
return;
553+
}
513554
mNavigator.getSimulator().unsetUserLocation();
514555
}
515556

516557
@ReactMethod
517558
public void pauseLocationSimulation() {
559+
if (mNavigator == null) {
560+
return;
561+
}
518562
mNavigator.getSimulator().pause();
519563
}
520564

521565
@ReactMethod
522566
public void resumeLocationSimulation() {
567+
if (mNavigator == null) {
568+
return;
569+
}
523570
mNavigator.getSimulator().resume();
524571
}
525572

@@ -530,6 +577,9 @@ public void setAbnormalTerminatingReportingEnabled(boolean isOn) {
530577

531578
@ReactMethod
532579
public void setSpeedAlertOptions(@Nullable ReadableMap options) {
580+
if (mNavigator == null) {
581+
return;
582+
}
533583
if (options == null) {
534584
mNavigator.setSpeedAlertOptions(null);
535585
return;
@@ -650,6 +700,14 @@ private void sendCommandToReactNative(String functionName, @Nullable Object para
650700
}
651701
}
652702

703+
private boolean ensureNavigatorAvailable(final Promise promise) {
704+
if (mNavigator == null) {
705+
promise.reject(JsErrors.NO_NAVIGATOR_ERROR_CODE, JsErrors.NO_NAVIGATOR_ERROR_MESSAGE);
706+
return false;
707+
}
708+
return true;
709+
}
710+
653711
@ReactMethod
654712
public void simulateLocation(ReadableMap location) {
655713
if (mNavigator != null) {

‎example/.detoxrc.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ module.exports = {
5959
type: 'ios.simulator',
6060
device: {
6161
type: 'iPhone 16 Pro',
62-
os: 'iOS 18.5',
62+
os: 'iOS 18.6',
6363
},
6464
},
6565
attached: {

‎example/e2e/event_listener.test.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ import {
2020
selectTestByName,
2121
waitForTestToFinish,
2222
expectSuccess,
23+
expectNoErrors,
2324
} from './shared.js';
2425

25-
describe('Even listener tests', () => {
26+
describe('Event listener tests', () => {
2627
beforeEach(async () => {
2728
await initializeIntegrationTestsPage();
2829
});
@@ -31,20 +32,23 @@ describe('Even listener tests', () => {
3132
await selectTestByName('testOnRemainingTimeOrDistanceChanged');
3233
await agreeToTermsAndConditions();
3334
await waitForTestToFinish();
35+
await expectNoErrors();
3436
await expectSuccess();
3537
});
3638

3739
it('T02 - test onArrival event listener', async () => {
3840
await selectTestByName('testOnArrival');
3941
await agreeToTermsAndConditions();
4042
await waitForTestToFinish();
43+
await expectNoErrors();
4144
await expectSuccess();
4245
});
4346

4447
it('T03 - test onRouteChanged event listener', async () => {
4548
await selectTestByName('testOnRouteChanged');
4649
await agreeToTermsAndConditions();
4750
await waitForTestToFinish();
51+
await expectNoErrors();
4852
await expectSuccess();
4953
});
5054
});

‎example/e2e/map.test.js‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import {
1919
selectTestByName,
2020
waitForTestToFinish,
2121
expectSuccess,
22+
expectNoErrors,
2223
} from './shared.js';
23-
import { element, by, log } from 'detox';
2424

2525
describe('Map view tests', () => {
2626
beforeEach(async () => {
@@ -30,30 +30,21 @@ describe('Map view tests', () => {
3030
it('T01 - initialize map and test default values', async () => {
3131
await selectTestByName('testMapInitialization');
3232
await waitForTestToFinish();
33-
const failureMessageLabel = element(by.id('failure_message_label'));
34-
const attributes = await failureMessageLabel.getAttributes();
35-
log.error(attributes.text);
36-
await expect(element(by.id('failure_message_label'))).toHaveText('');
33+
await expectNoErrors();
3734
await expectSuccess();
3835
});
3936

4037
it('T02 - initialize map and test move camera', async () => {
4138
await selectTestByName('testMoveCamera');
4239
await waitForTestToFinish();
43-
const failureMessageLabel = element(by.id('failure_message_label'));
44-
const attributes = await failureMessageLabel.getAttributes();
45-
log.error(attributes.text);
46-
await expect(element(by.id('failure_message_label'))).toHaveText('');
40+
await expectNoErrors();
4741
await expectSuccess();
4842
});
4943

5044
it('T03 - initialize map and test camera tilt bearing zoom', async () => {
5145
await selectTestByName('testTiltZoomBearingCamera');
5246
await waitForTestToFinish();
53-
const failureMessageLabel = element(by.id('failure_message_label'));
54-
const attributes = await failureMessageLabel.getAttributes();
55-
log.error(attributes.text);
56-
await expect(element(by.id('failure_message_label'))).toHaveText('');
47+
await expectNoErrors();
5748
await expectSuccess();
5849
});
5950
});

‎example/e2e/navigation.test.js‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import {
2020
selectTestByName,
2121
waitForTestToFinish,
2222
expectSuccess,
23+
expectNoErrors,
2324
} from './shared.js';
24-
import { element, by, log } from 'detox';
2525

2626
describe('Navigation tests', () => {
2727
beforeEach(async () => {
@@ -32,42 +32,54 @@ describe('Navigation tests', () => {
3232
await selectTestByName('testNavigationSessionInitialization');
3333
await agreeToTermsAndConditions();
3434
await waitForTestToFinish();
35+
await expectNoErrors();
3536
await expectSuccess();
3637
});
3738

3839
it('T02 - initialize navigation controller and navigate to single destination', async () => {
3940
await selectTestByName('testNavigationToSingleDestination');
4041
await agreeToTermsAndConditions();
4142
await waitForTestToFinish();
43+
await expectNoErrors();
4244
await expectSuccess();
4345
});
4446

4547
it('T03 - initialize navigation controller and navigate to multiple destinations', async () => {
4648
await selectTestByName('testNavigationToMultipleDestination');
4749
await agreeToTermsAndConditions();
4850
await waitForTestToFinish();
51+
await expectNoErrors();
4952
await expectSuccess();
5053
});
5154

5255
it('T04 - initialize navigation controller and test route segments', async () => {
5356
await selectTestByName('testRouteSegments');
5457
await agreeToTermsAndConditions();
55-
const failureMessageLabel = element(by.id('failure_message_label'));
56-
const attributes = await failureMessageLabel.getAttributes();
57-
log.error(attributes.text);
58-
await expect(element(by.id('failure_message_label'))).toHaveText('');
5958
await waitForTestToFinish();
59+
await expectNoErrors();
6060
await expectSuccess();
6161
});
6262

6363
it('T05 - initialize navigation controller and test remaining time and distance', async () => {
6464
await selectTestByName('testGetCurrentTimeAndDistance');
6565
await agreeToTermsAndConditions();
66-
const failureMessageLabel = element(by.id('failure_message_label'));
67-
const attributes = await failureMessageLabel.getAttributes();
68-
log.error(attributes.text);
69-
await expect(element(by.id('failure_message_label'))).toHaveText('');
7066
await waitForTestToFinish();
67+
await expectNoErrors();
68+
await expectSuccess();
69+
});
70+
71+
it('T06 - expect navigation controller calls to fail when not initialized', async () => {
72+
await selectTestByName('testNavigationStateGuards');
73+
await waitForTestToFinish();
74+
await expectNoErrors();
75+
await expectSuccess();
76+
});
77+
78+
it('T07 - require destinations before starting guidance', async () => {
79+
await selectTestByName('testStartGuidanceWithoutDestinations');
80+
await agreeToTermsAndConditions();
81+
await waitForTestToFinish();
82+
await expectNoErrors();
7183
await expectSuccess();
7284
});
7385
});

0 commit comments

Comments
 (0)