Skip to content

Commit 0fa416b

Browse files
yury-syury-s
authored
fix: manually pipe messages from child process sdtout/stderr (microsoft#426)
1 parent a95f8f3 commit 0fa416b

7 files changed

Lines changed: 105 additions & 6 deletions

File tree

‎driver-bundle/pom.xml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@
4444
<plugin>
4545
<groupId>org.apache.maven.plugins</groupId>
4646
<artifactId>maven-surefire-plugin</artifactId>
47-
<version>3.0.0-M5</version>
47+
<configuration>
48+
<redirectTestOutputToFile>true</redirectTestOutputToFile>
49+
</configuration>
4850
</plugin>
4951
</plugins>
5052
</build>

‎driver-bundle/src/main/java/com/microsoft/playwright/impl/DriverJar.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ private void installBrowsers() throws IOException, InterruptedException {
4848
p.destroy();
4949
throw new RuntimeException("Timed out waiting for browsers to install");
5050
}
51+
if (p.exitValue() != 0) {
52+
throw new RuntimeException("Failed to install browsers, exit code: " + p.exitValue());
53+
}
5154
}
5255

5356
private static boolean isExecutable(Path filePath) {

‎driver-bundle/src/test/java/com/microsoft/playwright/TestInstall.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.microsoft.playwright;
1818

1919
import com.microsoft.playwright.impl.Driver;
20+
import com.microsoft.playwright.impl.StreamRedirectThread;
2021
import org.junit.jupiter.api.Test;
2122

2223
import java.nio.file.Files;
@@ -36,11 +37,13 @@ void playwrightCliInstalled() throws Exception {
3637
assertTrue(Files.exists(cli));
3738

3839
ProcessBuilder pb = new ProcessBuilder(cli.toString(), "install");
39-
pb.redirectError(ProcessBuilder.Redirect.INHERIT);
40-
pb.redirectOutput(ProcessBuilder.Redirect.INHERIT);
4140
Process p = pb.start();
41+
StreamRedirectThread stdoutThread = new StreamRedirectThread(p.getInputStream(), System.out);
42+
StreamRedirectThread stderrThread = new StreamRedirectThread(p.getErrorStream(), System.err);
4243
boolean result = p.waitFor(1, TimeUnit.MINUTES);
4344
assertTrue(result, "Timed out waiting for browsers to install");
45+
stderrThread.terminateAndJoin();
46+
stdoutThread.terminateAndJoin();
4447
} catch (Exception e) {
4548
e.printStackTrace();
4649
assertNull(e);

‎driver/pom.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@
4040
<plugin>
4141
<groupId>org.apache.maven.plugins</groupId>
4242
<artifactId>maven-surefire-plugin</artifactId>
43+
<configuration>
44+
<redirectTestOutputToFile>true</redirectTestOutputToFile>
45+
</configuration>
4346
</plugin>
4447
</plugins>
4548
</build>

‎driver/src/main/java/com/microsoft/playwright/impl/StreamRedirectThread.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright (c) Microsoft Corporation.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.microsoft.playwright.impl;
18+
19+
import java.io.IOException;
20+
import java.io.InputStream;
21+
import java.io.OutputStream;
22+
23+
// We manually copy stderr and stdout from child process as INHERIT for err/out streams
24+
// doesn't work well in Java Enterprise, see
25+
// https://github.com/microsoft/playwright-java/issues/418#issuecomment-832650650
26+
public class StreamRedirectThread extends Thread {
27+
private final InputStream from;
28+
private final OutputStream to;
29+
private volatile boolean terminated;
30+
31+
public StreamRedirectThread(InputStream from, OutputStream to) {
32+
this.from = from;
33+
this.to = to;
34+
start();
35+
}
36+
37+
@Override
38+
public void run() {
39+
byte[] buffer = new byte[1<<14];
40+
try {
41+
while (true) {
42+
while (from.available() != 0) {
43+
int len = from.read(buffer);
44+
if (len != -1) {
45+
to.write(buffer);
46+
}
47+
}
48+
if (terminated) {
49+
break;
50+
}
51+
try {
52+
Thread.sleep(100);
53+
} catch (InterruptedException e) {
54+
}
55+
}
56+
} catch (IOException e) {
57+
e.printStackTrace(System.err);
58+
}
59+
}
60+
61+
public void terminateAndJoin() {
62+
terminated = true;
63+
try {
64+
join();
65+
} catch (InterruptedException e) {
66+
e.printStackTrace(System.err);
67+
}
68+
}
69+
}

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,16 @@ public String poll(Duration timeout) {
5959
throw new PlaywrightException("Playwright connection closed");
6060
}
6161
try {
62-
return incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS);
62+
String message = incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS);
63+
if (message == null && readerThread.exception != null) {
64+
try {
65+
close();
66+
} catch (IOException e) {
67+
e.printStackTrace(System.err);
68+
}
69+
throw new PlaywrightException("Failed to read message from driver, pipe closed.", readerThread.exception);
70+
}
71+
return message;
6372
} catch (InterruptedException e) {
6473
throw new PlaywrightException("Failed to read message", e);
6574
}
@@ -84,6 +93,7 @@ class ReaderThread extends Thread {
8493
private final DataInputStream in;
8594
private final BlockingQueue<String> queue;
8695
volatile boolean isClosing;
96+
volatile Exception exception;
8797

8898
private static int readIntLE(DataInputStream in) throws IOException {
8999
int ch1 = in.read();
@@ -109,7 +119,7 @@ public void run() {
109119
queue.put(readMessage());
110120
} catch (IOException e) {
111121
if (!isInterrupted() && !isClosing) {
112-
e.printStackTrace();
122+
exception = e;
113123
}
114124
break;
115125
} catch (InterruptedException e) {

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,29 @@
2727

2828
public class PlaywrightImpl extends ChannelOwner implements Playwright {
2929
private Process driverProcess;
30+
private StreamRedirectThread stderrThread;
3031

3132
public static PlaywrightImpl create() {
33+
StreamRedirectThread stderrThread = null;
3234
try {
3335
Path driver = Driver.ensureDriverInstalled();
3436
ProcessBuilder pb = new ProcessBuilder(driver.toString(), "run-driver");
35-
pb.redirectError(ProcessBuilder.Redirect.INHERIT);
3637
// pb.environment().put("DEBUG", "pw:pro*");
3738
Process p = pb.start();
39+
stderrThread = new StreamRedirectThread(p.getErrorStream(), System.err);
3840
Connection connection = new Connection(new PipeTransport(p.getInputStream(), p.getOutputStream()));
3941
PlaywrightImpl result = (PlaywrightImpl) connection.waitForObjectWithKnownName("Playwright");
4042
result.driverProcess = p;
43+
result.stderrThread = stderrThread;
44+
stderrThread = null;
4145
result.initSharedSelectors(null);
4246
return result;
4347
} catch (IOException e) {
4448
throw new PlaywrightException("Failed to launch driver", e);
49+
} finally {
50+
if (stderrThread != null) {
51+
stderrThread.terminateAndJoin();
52+
}
4553
}
4654
}
4755

@@ -103,6 +111,7 @@ public void close() {
103111
if (!didClose) {
104112
System.err.println("WARNING: Timed out while waiting for driver process to exit");
105113
}
114+
stderrThread.terminateAndJoin();
106115
} catch (IOException e) {
107116
throw new PlaywrightException("Failed to terminate", e);
108117
} catch (InterruptedException e) {

0 commit comments

Comments
 (0)