Skip to content

Commit 81e9281

Browse files
authored
fix(geolocation): always invoke callback / signal on JS-bridge failure (#24260)
Geolocation.get() and the watch path silently dropped the request when the JS bridge rejected (module not loaded, executeJs error, serialization mismatch). The application saw nothing — only a DEBUG log line. Now both paths surface the failure as a GeolocationError carrying the UNKNOWN error code, so apps see a consistent outcome via their existing callback or valueSignal.
1 parent e9c55d2 commit 81e9281

4 files changed

Lines changed: 68 additions & 3 deletions

File tree

flow-server/src/main/java/com/vaadin/flow/component/geolocation/BrowserGeolocationClient.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,13 @@ private final class BrowserWatchHandle implements WatchHandle {
171171
.addEventDetail().allowInert();
172172
el.executeJs("window.Vaadin.Flow.geolocation.watch(this, $0, $1)",
173173
options, watchKey).then(ignored -> {
174-
}, err -> LOGGER.debug(
175-
"Client-side geolocation.watch failed: {}", err));
174+
}, err -> {
175+
LOGGER.debug("Client-side geolocation.watch failed: {}",
176+
err);
177+
onUpdate.accept(new GeolocationError(
178+
GeolocationErrorCode.UNKNOWN.code(),
179+
"Client-side geolocation bridge failure"));
180+
});
176181
}
177182

178183
@Override

flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,9 @@ public void get(@Nullable GeolocationOptions options,
213213
client.get(options).whenComplete((outcome, error) -> {
214214
if (error != null) {
215215
LOGGER.debug("Geolocation get() failed", error);
216+
callback.accept(new GeolocationError(
217+
GeolocationErrorCode.UNKNOWN.code(),
218+
"Client-side geolocation bridge failure"));
216219
} else {
217220
callback.accept(outcome);
218221
}

flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.ArrayList;
1919
import java.util.List;
2020
import java.util.concurrent.CompletableFuture;
21+
import java.util.concurrent.atomic.AtomicReference;
2122

2223
import org.jspecify.annotations.Nullable;
2324
import org.junit.jupiter.api.BeforeEach;
@@ -34,6 +35,8 @@
3435
import com.vaadin.tests.util.MockUI;
3536

3637
import static org.junit.jupiter.api.Assertions.assertEquals;
38+
import static org.junit.jupiter.api.Assertions.assertFalse;
39+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
3740
import static org.junit.jupiter.api.Assertions.assertNotNull;
3841
import static org.junit.jupiter.api.Assertions.assertNull;
3942
import static org.junit.jupiter.api.Assertions.assertSame;
@@ -127,6 +130,29 @@ void track_handleIsNullAfterStop() {
127130
"handle() should return null after stop()");
128131
}
129132

133+
@Test
134+
void get_callbackReceivesUnknownErrorWhenClientFutureFailsExceptionally() {
135+
FakeClient fake = new FakeClient();
136+
fake.nextGetResult = CompletableFuture
137+
.failedFuture(new RuntimeException(
138+
"Client-side geolocation.get failed: boom"));
139+
ui.getGeolocation().setClient(fake);
140+
141+
AtomicReference<@Nullable GeolocationOutcome> received = new AtomicReference<>();
142+
ui.getGeolocation().get(received::set);
143+
144+
GeolocationOutcome outcome = received.get();
145+
assertNotNull(outcome,
146+
"callback must fire even when the JS bridge fails");
147+
GeolocationError err = assertInstanceOf(GeolocationError.class, outcome,
148+
"infra failure should surface as a GeolocationError");
149+
assertEquals(GeolocationErrorCode.UNKNOWN, err.errorCode(),
150+
"error code should be UNKNOWN for client-bridge failures");
151+
assertFalse(err.message().contains("boom"),
152+
"synthesized message must not leak the wrapped exception text;"
153+
+ " the cause is logged at DEBUG instead");
154+
}
155+
130156
/**
131157
* Minimal in-test fake. Records get() calls and serves a sentinel
132158
* WatchHandle from startWatch.
@@ -135,12 +161,15 @@ private static class FakeClient implements GeolocationClient {
135161
final List<@Nullable GeolocationOptions> getCalls = new ArrayList<>();
136162
boolean closed;
137163
GeolocationClient.@Nullable WatchHandle lastWatchHandle;
164+
@Nullable
165+
CompletableFuture<GeolocationOutcome> nextGetResult;
138166

139167
@Override
140168
public CompletableFuture<GeolocationOutcome> get(
141169
@Nullable GeolocationOptions options) {
142170
getCalls.add(options);
143-
return new CompletableFuture<>();
171+
CompletableFuture<GeolocationOutcome> result = nextGetResult;
172+
return result != null ? result : new CompletableFuture<>();
144173
}
145174

146175
@Override

flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,34 @@ void track_registersListenersAndExecutesWatchJs() {
230230
.getExpression().contains("geolocation.watch")));
231231
}
232232

233+
@Test
234+
void track_signalSurfacesUnknownErrorWhenWatchExecuteJsFails() {
235+
TestComponent component = new TestComponent();
236+
ui.add(component);
237+
238+
GeolocationTracker tracker = ui.getGeolocation().track(component);
239+
240+
PendingJavaScriptInvocation watchInvocation = ui
241+
.dumpPendingJsInvocations().stream()
242+
.filter(inv -> inv.getInvocation().getExpression()
243+
.contains("geolocation.watch"))
244+
.reduce((a, b) -> b).orElseThrow();
245+
watchInvocation.completeExceptionally(
246+
JacksonUtils.createNode("module not loaded"));
247+
248+
GeolocationResult value = tracker.valueSignal().peek();
249+
GeolocationError err = assertInstanceOf(GeolocationError.class, value,
250+
"watch executeJs failure must surface as a GeolocationError"
251+
+ " on the tracker's valueSignal");
252+
assertEquals(GeolocationErrorCode.UNKNOWN, err.errorCode());
253+
assertFalse(err.message().contains("module not loaded"),
254+
"synthesized message must not leak the wrapped client text;"
255+
+ " the cause is logged at DEBUG instead");
256+
assertTrue(tracker.activeSignal().peek(),
257+
"activeSignal stays true on infra failure — its contract"
258+
+ " is tied to resume()/stop(), not data flow");
259+
}
260+
233261
@Test
234262
void track_signalUpdatesOnPositionEvent() {
235263
TestComponent component = new TestComponent();

0 commit comments

Comments
 (0)