Skip to content

Commit 859eb9b

Browse files
yury-syury-s
authored
chore: require explicit timeout parameter in sendMessage() (#1807)
1 parent f28cb55 commit 859eb9b

29 files changed

+195
-233
lines changed

‎playwright/src/main/java/com/microsoft/playwright/impl/APIRequestContextImpl.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void dispose(DisposeOptions options) {
5858
}
5959
disposeReason = options.reason;
6060
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
61-
sendMessage("dispose", params);
61+
sendMessage("dispose", params, NO_TIMEOUT);
6262
}
6363

6464
@Override
@@ -131,7 +131,6 @@ private APIResponse fetchImpl(String url, RequestOptionsImpl options) {
131131
if (options.multipart != null) {
132132
params.add("multipartData", serializeMultipartData(options.multipart.fields));
133133
}
134-
params.addProperty("timeout", timeoutSettings.timeout(options.timeout));
135134
if (options.failOnStatusCode != null) {
136135
params.addProperty("failOnStatusCode", options.failOnStatusCode);
137136
}
@@ -150,7 +149,7 @@ private APIResponse fetchImpl(String url, RequestOptionsImpl options) {
150149
}
151150
params.addProperty("maxRetries", options.maxRetries);
152151
}
153-
JsonObject json = sendMessage("fetch", params).getAsJsonObject();
152+
JsonObject json = sendMessage("fetch", params, timeoutSettings.timeout(options.timeout)).getAsJsonObject();
154153
return new APIResponseImpl(this, json.getAsJsonObject("response"));
155154
}
156155

‎playwright/src/main/java/com/microsoft/playwright/impl/APIRequestImpl.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.nio.file.Files;
2828
import java.util.List;
2929

30+
import static com.microsoft.playwright.impl.ChannelOwner.NO_TIMEOUT;
3031
import static com.microsoft.playwright.impl.Serialization.gson;
3132
import static com.microsoft.playwright.impl.Utils.addToProtocol;
3233

@@ -60,14 +61,17 @@ public APIRequestContextImpl newContext(NewContextOptions options) {
6061
}
6162
List<ClientCertificate> clientCertificateList = options.clientCertificates;
6263
options.clientCertificates = null;
64+
Double timeout = options.timeout;
65+
// Timeout is handled on the client.
66+
options.timeout = null;
6367
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
6468
if (storageState != null) {
6569
params.add("storageState", storageState);
6670
}
6771
addToProtocol(params, clientCertificateList);
68-
JsonObject result = playwright.sendMessage("newRequest", params).getAsJsonObject();
72+
JsonObject result = playwright.sendMessage("newRequest", params, NO_TIMEOUT).getAsJsonObject();
6973
APIRequestContextImpl context = playwright.connection.getExistingObject(result.getAsJsonObject("request").get("guid").getAsString());
70-
context.timeoutSettings.setDefaultTimeout(options.timeout);
74+
context.timeoutSettings.setDefaultTimeout(timeout);
7175
return context;
7276
}
7377
}

‎playwright/src/main/java/com/microsoft/playwright/impl/APIResponseImpl.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.List;
2929
import java.util.Map;
3030

31+
import static com.microsoft.playwright.impl.ChannelOwner.NO_TIMEOUT;
3132
import static com.microsoft.playwright.impl.Serialization.gson;
3233
import static com.microsoft.playwright.impl.Utils.isSafeCloseError;
3334
import static java.util.Arrays.asList;
@@ -48,7 +49,7 @@ public byte[] body() {
4849
try {
4950
JsonObject params = new JsonObject();
5051
params.addProperty("fetchUid", fetchUid());
51-
JsonObject json = context.sendMessage("fetchResponseBody", params).getAsJsonObject();
52+
JsonObject json = context.sendMessage("fetchResponseBody", params, NO_TIMEOUT).getAsJsonObject();
5253
if (!json.has("binary")) {
5354
throw new PlaywrightException("Response has been disposed");
5455
}
@@ -65,7 +66,7 @@ public byte[] body() {
6566
public void dispose() {
6667
JsonObject params = new JsonObject();
6768
params.addProperty("fetchUid", fetchUid());
68-
context.sendMessage("disposeAPIResponse", params);
69+
context.sendMessage("disposeAPIResponse", params, NO_TIMEOUT);
6970
}
7071

7172
@Override
@@ -111,7 +112,7 @@ String fetchUid() {
111112
List<String> fetchLog() {
112113
JsonObject params = new JsonObject();
113114
params.addProperty("fetchUid", fetchUid());
114-
JsonObject json = context.sendMessage("fetchLog", params).getAsJsonObject();
115+
JsonObject json = context.sendMessage("fetchLog", params, NO_TIMEOUT).getAsJsonObject();
115116
JsonArray log = json.get("log").getAsJsonArray();
116117
return gson().fromJson(log, new TypeToken<List<String>>() {}.getType());
117118
}

‎playwright/src/main/java/com/microsoft/playwright/impl/ArtifactImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,6 @@ public void saveAs(Path path) {
8686

8787
JsonObject params = new JsonObject();
8888
params.addProperty("path", path.toString());
89-
sendMessage("saveAs", params);
89+
sendMessage("saveAs", params, NO_TIMEOUT);
9090
}
9191
}

‎playwright/src/main/java/com/microsoft/playwright/impl/BindingCall.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ void call(BindingCallback binding) {
7878

7979
JsonObject params = new JsonObject();
8080
params.add("result", gson().toJsonTree(serializeArgument(result)));
81-
sendMessage("resolve", params);
81+
sendMessage("resolve", params, NO_TIMEOUT);
8282
} catch (RuntimeException exception) {
8383
JsonObject params = new JsonObject();
8484
params.add("error", gson().toJsonTree(serializeError(exception)));
85-
sendMessage("reject", params);
85+
sendMessage("reject", params, NO_TIMEOUT);
8686
}
8787
}
8888
}

‎playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ enum EventType {
100100
request = connection.getExistingObject(initializer.getAsJsonObject("requestContext").get("guid").getAsString());
101101
request.timeoutSettings = timeoutSettings;
102102
clock = new ClockImpl(this);
103-
closePromise = new WaitableEvent<>(listeners, EventType.CLOSE);
103+
closePromise = new WaitableEvent<>(listeners, EventType.CLOSE);
104104
}
105105

106106
Path videosDir() {
@@ -121,7 +121,7 @@ URL baseUrl() {
121121
}
122122
return null;
123123
}
124-
124+
125125
String effectiveCloseReason() {
126126
if (closeReason != null) {
127127
return closeReason;
@@ -261,15 +261,15 @@ private Page waitForPageImpl(WaitForPageOptions options, Runnable code) {
261261
public CDPSession newCDPSession(Page page) {
262262
JsonObject params = new JsonObject();
263263
params.add("page", ((PageImpl) page).toProtocolRef());
264-
JsonObject result = sendMessage("newCDPSession", params).getAsJsonObject();
264+
JsonObject result = sendMessage("newCDPSession", params, NO_TIMEOUT).getAsJsonObject();
265265
return connection.getExistingObject(result.getAsJsonObject("session").get("guid").getAsString());
266266
}
267267

268268
@Override
269269
public CDPSession newCDPSession(Frame frame) {
270270
JsonObject params = new JsonObject();
271271
params.add("frame", ((FrameImpl) frame).toProtocolRef());
272-
JsonObject result = sendMessage("newCDPSession", params).getAsJsonObject();
272+
JsonObject result = sendMessage("newCDPSession", params, NO_TIMEOUT).getAsJsonObject();
273273
return connection.getExistingObject(result.getAsJsonObject("session").get("guid").getAsString());
274274
}
275275

@@ -285,7 +285,7 @@ public void close(CloseOptions options) {
285285
for (Map.Entry<String, HarRecorder> entry : harRecorders.entrySet()) {
286286
JsonObject params = new JsonObject();
287287
params.addProperty("harId", entry.getKey());
288-
JsonObject json = sendMessage("harExport", params).getAsJsonObject();
288+
JsonObject json = sendMessage("harExport", params, NO_TIMEOUT).getAsJsonObject();
289289
ArtifactImpl artifact = connection.getExistingObject(json.getAsJsonObject("artifact").get("guid").getAsString());
290290
// Server side will compress artifact if content is attach or if file is .zip.
291291
HarRecorder harParams = entry.getValue();
@@ -297,14 +297,14 @@ public void close(CloseOptions options) {
297297
JsonObject unzipParams = new JsonObject();
298298
unzipParams.addProperty("zipFile", tmpPath);
299299
unzipParams.addProperty("harFile", harParams.path.toString());
300-
connection.localUtils.sendMessage("harUnzip", unzipParams);
300+
connection.localUtils.sendMessage("harUnzip", unzipParams, NO_TIMEOUT);
301301
} else {
302302
artifact.saveAs(harParams.path);
303303
}
304304
artifact.delete();
305305
}
306306
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
307-
sendMessage("close", params);
307+
sendMessage("close", params, NO_TIMEOUT);
308308
}
309309
runUntil(() -> {}, closePromise);
310310
}
@@ -319,14 +319,14 @@ public List<Cookie> cookies(String url) {
319319
public void addCookies(List<Cookie> cookies) {
320320
JsonObject params = new JsonObject();
321321
params.add("cookies", gson().toJsonTree(cookies));
322-
sendMessage("addCookies", params);
322+
sendMessage("addCookies", params, NO_TIMEOUT);
323323
}
324324

325325
@Override
326326
public void addInitScript(String script) {
327327
JsonObject params = new JsonObject();
328328
params.addProperty("source", script);
329-
sendMessage("addInitScript", params);
329+
sendMessage("addInitScript", params, NO_TIMEOUT);
330330
}
331331

332332
@Override
@@ -358,7 +358,7 @@ public void clearCookies(ClearCookiesOptions options) {
358358
setStringOrRegex(params, "name", options.name);
359359
setStringOrRegex(params, "domain", options.domain);
360360
setStringOrRegex(params, "path", options.path);
361-
sendMessage("clearCookies", params);
361+
sendMessage("clearCookies", params, NO_TIMEOUT);
362362
}
363363

364364
private static void setStringOrRegex(JsonObject params, String name, Object value) {
@@ -383,7 +383,7 @@ public List<Cookie> cookies(List<String> urls) {
383383
urls = new ArrayList<>();
384384
}
385385
params.add("urls", gson().toJsonTree(urls));
386-
JsonObject json = sendMessage("cookies", params).getAsJsonObject();
386+
JsonObject json = sendMessage("cookies", params, NO_TIMEOUT).getAsJsonObject();
387387
Cookie[] cookies = gson().fromJson(json.getAsJsonArray("cookies"), Cookie[].class);
388388
return asList(cookies);
389389
}
@@ -409,7 +409,7 @@ private void exposeBindingImpl(String name, BindingCallback playwrightBinding, E
409409
if (options != null && options.handle != null && options.handle) {
410410
params.addProperty("needsHandle", true);
411411
}
412-
sendMessage("exposeBinding", params);
412+
sendMessage("exposeBinding", params, NO_TIMEOUT);
413413
}
414414

415415
@Override
@@ -427,7 +427,7 @@ public void grantPermissions(List<String> permissions, GrantPermissionsOptions o
427427
}
428428
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
429429
params.add("permissions", gson().toJsonTree(permissions));
430-
sendMessage("grantPermissions", params);
430+
sendMessage("grantPermissions", params, NO_TIMEOUT);
431431
}
432432

433433
@Override
@@ -517,13 +517,13 @@ void recordIntoHar(PageImpl page, Path har, RouteFromHAROptions options, HarCont
517517
params.add("page", page.toProtocolRef());
518518
}
519519
JsonObject recordHarArgs = new JsonObject();
520-
recordHarArgs.addProperty("zip", har.toString().endsWith(".zip"));
520+
recordHarArgs.addProperty("zip", har.toString().endsWith(".zip"));
521521
recordHarArgs.addProperty("content", contentPolicy.name().toLowerCase());
522522
recordHarArgs.addProperty("mode", (options.updateMode == null ? HarMode.MINIMAL : options.updateMode).name().toLowerCase());
523523
addHarUrlFilter(recordHarArgs, options.url);
524524

525525
params.add("options", recordHarArgs);
526-
JsonObject json = sendMessage("harStart", params).getAsJsonObject();
526+
JsonObject json = sendMessage("harStart", params, NO_TIMEOUT).getAsJsonObject();
527527
String harId = json.get("harId").getAsString();
528528
harRecorders.put(harId, new HarRecorder(har, contentPolicy));
529529
}
@@ -549,7 +549,7 @@ public void setExtraHTTPHeaders(Map<String, String> headers) {
549549
jsonHeaders.add(header);
550550
}
551551
params.add("headers", jsonHeaders);
552-
sendMessage("setExtraHTTPHeaders", params);
552+
sendMessage("setExtraHTTPHeaders", params, NO_TIMEOUT);
553553
}
554554

555555
@Override
@@ -558,14 +558,14 @@ public void setGeolocation(Geolocation geolocation) {
558558
if (geolocation != null) {
559559
params.add("geolocation", gson().toJsonTree(geolocation));
560560
}
561-
sendMessage("setGeolocation", params);
561+
sendMessage("setGeolocation", params, NO_TIMEOUT);
562562
}
563563

564564
@Override
565565
public void setOffline(boolean offline) {
566566
JsonObject params = new JsonObject();
567567
params.addProperty("offline", offline);
568-
sendMessage("setOffline", params);
568+
sendMessage("setOffline", params, NO_TIMEOUT);
569569
}
570570

571571
@Override
@@ -575,7 +575,7 @@ public String storageState(StorageStateOptions options) {
575575
}
576576
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
577577
params.remove("path");
578-
JsonElement json = sendMessage("storageState", params);
578+
JsonElement json = sendMessage("storageState", params, NO_TIMEOUT);
579579

580580
String storageState = json.toString();
581581
if (options.path != null) {
@@ -648,11 +648,11 @@ private void unroute(UrlMatcher matcher, Consumer<Route> handler) {
648648
}
649649

650650
private void updateInterceptionPatterns() {
651-
sendMessage("setNetworkInterceptionPatterns", routes.interceptionPatterns());
651+
sendMessage("setNetworkInterceptionPatterns", routes.interceptionPatterns(), NO_TIMEOUT);
652652
}
653653

654654
private void updateWebSocketInterceptionPatterns() {
655-
sendMessage("setWebSocketInterceptionPatterns", webSocketRoutes.interceptionPatterns());
655+
sendMessage("setWebSocketInterceptionPatterns", webSocketRoutes.interceptionPatterns(), NO_TIMEOUT);
656656
}
657657

658658
void handleRoute(RouteImpl route) {
@@ -817,7 +817,7 @@ WritableStream createTempFile(String name, long lastModifiedMs) {
817817
JsonObject params = new JsonObject();
818818
params.addProperty("name", name);
819819
params.addProperty("lastModifiedMs", lastModifiedMs);
820-
JsonObject json = sendMessage("createTempFile", params).getAsJsonObject();
820+
JsonObject json = sendMessage("createTempFile", params, NO_TIMEOUT).getAsJsonObject();
821821
return connection.getExistingObject(json.getAsJsonObject("writableStream").get("guid").getAsString());
822822
}
823823

‎playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public BrowserContextImpl newContext(NewContextOptions options) {
123123
options.recordHarPath = null;
124124
options.recordHarOmitContent = null;
125125
options.recordHarUrlFilter = null;
126-
126+
127127
if (options.storageStatePath != null) {
128128
try {
129129
byte[] bytes = Files.readAllBytes(options.storageStatePath);
@@ -171,7 +171,7 @@ public BrowserContextImpl newContext(NewContextOptions options) {
171171
}
172172
params.add("selectorEngines", gson().toJsonTree(browserType.playwright.selectors.selectorEngines));
173173
params.addProperty("testIdAttributeName", browserType.playwright.selectors.testIdAttributeName);
174-
JsonElement result = sendMessage("newContext", params);
174+
JsonElement result = sendMessage("newContext", params, NO_TIMEOUT);
175175
BrowserContextImpl context = connection.getExistingObject(result.getAsJsonObject().getAsJsonObject("context").get("guid").getAsString());
176176
context.initializeHarFromOptions(harOptions);
177177
return context;
@@ -192,7 +192,7 @@ public void startTracing(Page page, StartTracingOptions options) {
192192
if (page != null) {
193193
params.add("page", ((PageImpl) page).toProtocolRef());
194194
}
195-
sendMessage("startTracing", params);
195+
sendMessage("startTracing", params, NO_TIMEOUT);
196196
}
197197

198198
@Override
@@ -250,7 +250,7 @@ void handleEvent(String event, JsonObject parameters) {
250250
@Override
251251
public CDPSession newBrowserCDPSession() {
252252
JsonObject params = new JsonObject();
253-
JsonObject result = sendMessage("newBrowserCDPSession", params).getAsJsonObject();
253+
JsonObject result = sendMessage("newBrowserCDPSession", params, NO_TIMEOUT).getAsJsonObject();
254254
return connection.getExistingObject(result.getAsJsonObject("session").get("guid").getAsString());
255255
}
256256

‎playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ public BrowserImpl launch(LaunchOptions options) {
4444
if (options == null) {
4545
options = new LaunchOptions();
4646
}
47-
options.timeout = TimeoutSettings.launchTimeout(options.timeout);
4847
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
49-
JsonElement result = sendMessage("launch", params);
48+
JsonElement result = sendMessage("launch", params, TimeoutSettings.launchTimeout(options.timeout));
5049
BrowserImpl browser = connection.getExistingObject(result.getAsJsonObject().getAsJsonObject("browser").get("guid").getAsString());
5150
browser.browserType = this;
5251
browser.launchOptions = options;
@@ -77,11 +76,12 @@ public Browser connect(String wsEndpoint, ConnectOptions options) {
7776
headers.addProperty("x-playwright-browser", name());
7877
}
7978

80-
if (!params.has("timeout")) {
81-
params.addProperty("timeout", 0);
79+
Double timeout = options.timeout;
80+
if (timeout == null) {
81+
timeout = 0.0;
8282
}
8383

84-
JsonObject json = connection.localUtils().sendMessage("connect", params).getAsJsonObject();
84+
JsonObject json = connection.localUtils().sendMessage("connect", params, timeout).getAsJsonObject();
8585
JsonPipe pipe = connection.getExistingObject(json.getAsJsonObject("pipe").get("guid").getAsString());
8686
Connection connection = new Connection(pipe, this.connection.env, this.connection.localUtils);
8787
PlaywrightImpl playwright = connection.initializePlaywright();
@@ -118,11 +118,10 @@ public Browser connectOverCDP(String endpointURL, ConnectOverCDPOptions options)
118118
if (options == null) {
119119
options = new ConnectOverCDPOptions();
120120
}
121-
options.timeout = TimeoutSettings.launchTimeout(options.timeout);
122121

123122
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
124123
params.addProperty("endpointURL", endpointURL);
125-
JsonObject json = sendMessage("connectOverCDP", params).getAsJsonObject();
124+
JsonObject json = sendMessage("connectOverCDP", params, TimeoutSettings.launchTimeout(options.timeout)).getAsJsonObject();
126125

127126
BrowserImpl browser = connection.getExistingObject(json.getAsJsonObject("browser").get("guid").getAsString());
128127
browser.connectToBrowserType(this, null);
@@ -149,8 +148,6 @@ public BrowserContextImpl launchPersistentContext(Path userDataDir, LaunchPersis
149148
options.recordHarOmitContent = null;
150149
options.recordHarUrlFilter = null;
151150

152-
options.timeout = TimeoutSettings.launchTimeout(options.timeout);
153-
154151
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
155152
if (!userDataDir.isAbsolute() && !userDataDir.toString().isEmpty()) {
156153
Path cwd = Paths.get("").toAbsolutePath();
@@ -186,7 +183,7 @@ public BrowserContextImpl launchPersistentContext(Path userDataDir, LaunchPersis
186183
}
187184
params.add("selectorEngines", gson().toJsonTree(playwright.selectors.selectorEngines));
188185
params.addProperty("testIdAttributeName", playwright.selectors.testIdAttributeName);
189-
JsonObject json = sendMessage("launchPersistentContext", params).getAsJsonObject();
186+
JsonObject json = sendMessage("launchPersistentContext", params, TimeoutSettings.launchTimeout(options.timeout)).getAsJsonObject();
190187
BrowserImpl browser = connection.getExistingObject(json.getAsJsonObject("browser").get("guid").getAsString());
191188
browser.connectToBrowserType(this, options.tracesDir);
192189
BrowserContextImpl context = connection.getExistingObject(json.getAsJsonObject("context").get("guid").getAsString());

0 commit comments

Comments
 (0)