Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the watch process more consistant in case of errors #111

Merged
merged 1 commit into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/io/mvnpm/esbuild/BuildEventListener.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package io.mvnpm.esbuild;

import java.util.Optional;
import io.mvnpm.esbuild.model.WatchBuildResult;

public interface BuildEventListener {

void onBuild(Optional<BundleException> error);
void onBuild(WatchBuildResult result);

}
13 changes: 5 additions & 8 deletions src/main/java/io/mvnpm/esbuild/Bundler.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
import java.util.logging.Logger;

import io.mvnpm.esbuild.install.WebDepsInstaller;
import io.mvnpm.esbuild.model.BundleOptions;
import io.mvnpm.esbuild.model.BundleResult;
import io.mvnpm.esbuild.model.EsBuildConfig;
import io.mvnpm.esbuild.model.ExecuteResult;
import io.mvnpm.esbuild.model.*;
import io.mvnpm.esbuild.resolve.Resolver;

public class Bundler {
Expand Down Expand Up @@ -91,8 +88,8 @@ public static Watch watch(BundleOptions bundleOptions, BuildEventListener eventL
final Path dist = workDir.resolve(out);
final EsBuildConfig esBuildConfig = prepareForBundling(bundleOptions, workDir, dist, true);

final Process process = esBuild(workDir, esBuildConfig, eventListener);
return new Watch(process, workDir, dist);
final WatchStartResult r = esBuildWatch(workDir, esBuildConfig, eventListener);
return new Watch(r.process(), workDir, dist, r.firstBuildResult());
}

private static Path getWorkDir(BundleOptions bundleOptions) throws IOException {
Expand All @@ -115,10 +112,10 @@ public static void clearDependencies(Path nodeModulesDir) throws IOException {
deleteRecursive(nodeModulesDir);
}

protected static Process esBuild(Path workDir, EsBuildConfig esBuildConfig, BuildEventListener listener)
protected static WatchStartResult esBuildWatch(Path workDir, EsBuildConfig esBuildConfig, BuildEventListener listener)
throws IOException {
final Execute execute = getExecute(workDir, esBuildConfig);
return execute.execute(listener);
return execute.watch(listener);
}

protected static ExecuteResult esBuild(Path workDir, EsBuildConfig esBuildConfig) throws IOException {
Expand Down
79 changes: 60 additions & 19 deletions src/main/java/io/mvnpm/esbuild/Execute.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,20 @@
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BooleanSupplier;
import java.util.function.Consumer;
import java.util.logging.Level;
import java.util.logging.Logger;

import io.mvnpm.esbuild.model.EsBuildConfig;
import io.mvnpm.esbuild.model.ExecuteResult;
import io.mvnpm.esbuild.model.WatchBuildResult;
import io.mvnpm.esbuild.model.WatchStartResult;

public class Execute {

Expand Down Expand Up @@ -65,8 +70,36 @@ public ExecuteResult executeAndWait() throws IOException {
}
}

public Process execute(BuildEventListener listener) throws IOException {
return createProcess(getCommand(), Optional.of(listener));
public WatchStartResult watch(BuildEventListener listener) throws IOException {
final Process process = createProcess(getCommand(), Optional.of(listener));
try {
final InputStream processStream = process.getInputStream();
CountDownLatch latch = new CountDownLatch(1);
final AtomicReference<WatchBuildResult> result = new AtomicReference<>();
EXECUTOR_STREAMER.execute(new Streamer(process::isAlive, processStream, (r) -> {
if (latch.getCount() == 1) {
result.set(r);
latch.countDown();
} else {
listener.onBuild(r);
}
}, r -> {
if (latch.getCount() == 1) {
result.set(r);
latch.countDown();
} else if (!r.isSuccess()) {
listener.onBuild(r);
}
}));
latch.await();
if (!process.isAlive() && !result.get().isSuccess()) {
throw result.get().bundleException();
}
return new WatchStartResult(result.get(), process);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
}
}

private String[] getCommand() {
Expand All @@ -91,38 +124,43 @@ private String[] getCommand(String[] args) {
}

public Process createProcess(final String[] command, final Optional<BuildEventListener> listener) throws IOException {
Process process = new ProcessBuilder().redirectErrorStream(listener.isPresent()).directory(workDir.toFile())
return new ProcessBuilder().redirectErrorStream(listener.isPresent()).directory(workDir.toFile())
.command(command).start();
final InputStream errorStream = process.getInputStream();
listener.ifPresent(
buildEventListener -> EXECUTOR_STREAMER
.execute(new Streamer(process::isAlive, errorStream, buildEventListener)));
return process;
}

private record Streamer(BooleanSupplier isAlive, InputStream processStream,
BuildEventListener listener) implements Runnable {
BuildEventListener listener, Consumer<WatchBuildResult> onExit) implements Runnable {

@Override
public void run() {
final StringBuilder errorBuilder = new StringBuilder();
final AtomicBoolean hasError = new AtomicBoolean();
final StringBuilder outputBuilder = new StringBuilder();
consumeStream(isAlive, processStream, l -> {
logger.fine(l);
outputBuilder.append("\n").append(l);
if (l.contains("build finished")) {
logger.fine("Build finished!");
final String error = errorBuilder.toString();
errorBuilder.setLength(0);
final String output = outputBuilder.toString();
final boolean error = hasError.getAndSet(false);
outputBuilder.setLength(0);
EXECUTOR_BUILD_LISTENERS.execute(() -> {
if (error.isEmpty()) {
listener.onBuild(Optional.empty());
if (!error) {
listener.onBuild(new WatchBuildResult(output));
} else {
listener.onBuild(Optional.of(new BundleException("Error during bundling", error)));
listener.onBuild(
new WatchBuildResult(output, new BundleException("Error during bundling", output)));
}
});
} else if (l.contains("[ERROR]") || !errorBuilder.isEmpty()) {
errorBuilder.append("\n").append(l);
} else if (l.contains("[ERROR]")) {
hasError.set(true);
}
});
if (!hasError.get()) {
onExit.accept(new WatchBuildResult(outputBuilder.toString()));
} else {
onExit.accept(new WatchBuildResult(outputBuilder.toString(),
new BundleException("Process exited with error", outputBuilder.toString())));
}
}
}

Expand All @@ -132,13 +170,16 @@ private static String readStream(InputStream stream) {
return s.toString();
}

private static void consumeStream(BooleanSupplier shouldStop, InputStream stream, Consumer<String> newLineConsumer) {
private static void consumeStream(BooleanSupplier stayAlive, InputStream stream, Consumer<String> newLineConsumer) {
try (
final InputStreamReader in = new InputStreamReader(stream, StandardCharsets.UTF_8);
final BufferedReader reader = new BufferedReader(in)) {
String line;
while ((line = reader.readLine()) != null && shouldStop.getAsBoolean()) {
while ((line = reader.readLine()) != null) {
newLineConsumer.accept(line);
if (!stayAlive.getAsBoolean()) {
break;
}
}
} catch (IOException e) {
// ignore
Expand Down
25 changes: 24 additions & 1 deletion src/main/java/io/mvnpm/esbuild/Watch.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.List;

import io.mvnpm.esbuild.model.EntryPoint;
import io.mvnpm.esbuild.model.WatchBuildResult;

public class Watch {

Expand All @@ -13,10 +14,13 @@ public class Watch {

private final Path dist;

public Watch(Process process, Path workDir, Path dist) {
private final WatchBuildResult firstBuildResult;

public Watch(Process process, Path workDir, Path dist, WatchBuildResult firstBuildResult) {
this.process = process;
this.workDir = workDir;
this.dist = dist;
this.firstBuildResult = firstBuildResult;
}

public void updateEntries(List<EntryPoint> entries) throws IOException {
Expand All @@ -25,12 +29,31 @@ public void updateEntries(List<EntryPoint> entries) throws IOException {

public void stop() {
process.destroy();

}

public void waitForStop() {
process.destroy();
try {
process.waitFor();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
}
}

public Path workDir() {
return workDir;
}

public WatchBuildResult firstBuildResult() {
return firstBuildResult;
}

public boolean isAlive() {
return process.isAlive();
}

public Path dist() {
return dist;
}
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/io/mvnpm/esbuild/model/WatchBuildResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package io.mvnpm.esbuild.model;

import io.mvnpm.esbuild.BundleException;

public record WatchBuildResult(String output, BundleException bundleException) {
public WatchBuildResult(String output) {
this(output, null);
}

public boolean isSuccess() {
return bundleException == null;
}
}
4 changes: 4 additions & 0 deletions src/main/java/io/mvnpm/esbuild/model/WatchStartResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package io.mvnpm.esbuild.model;

public record WatchStartResult(WatchBuildResult firstBuildResult, Process process) {
}
45 changes: 31 additions & 14 deletions src/test/java/io/mvnpm/esbuild/BundlerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ public void shouldBundle() throws URISyntaxException, IOException {
executeTest(List.of("/webjars/htmx.org-1.8.4.jar"), WebDependencyType.WEBJARS, "application-webjar.js", true);
}

@Test
public void shouldWatchExitWithError() throws URISyntaxException, IOException, InterruptedException {
final BundleOptions options = getBundleOptions(List.of("/mvnpm/stimulus-3.2.1.jar"),
WebDependencyType.MVNPM,
"application-mvnpm.js").withEsConfig(EsBuildConfig.builder().fixedEntryNames().define("foo", "\"bar").build())
.build();

assertThrows(BundleException.class, () -> {
Bundler.watch(options, (r) -> {
}, true);
});
}

@Test
public void shouldWatch() throws URISyntaxException, IOException, InterruptedException {
// given
Expand All @@ -75,29 +88,32 @@ public void shouldWatch() throws URISyntaxException, IOException, InterruptedExc
// when
AtomicReference<CountDownLatch> latch = new AtomicReference<>(new CountDownLatch(1));
AtomicReference<BundleException> bundleException = new AtomicReference<>();
final Watch watch = Bundler.watch(options, (error) -> {
error.ifPresent(bundleException::set);
final Watch watch = Bundler.watch(options, (r) -> {
if (!r.isSuccess()) {
bundleException.set(r.bundleException());
}
latch.get().countDown();
}, true);

// then
assertTrue(latch.get().await(2, TimeUnit.SECONDS));
assertNull(bundleException.get(), "No error during bundling");
assertTrue(latch.get().getCount() == 1, "First build is not using the listener");
assertTrue(watch.firstBuildResult().isSuccess(), "first build is success");
assertTrue(watch.isAlive(), "process is alive");
final Path app = watch.workDir().resolve("application-mvnpm.js");
assertTrue(Files.exists(app));
final Path distApp = watch.dist().resolve("application-mvnpm.js");
assertTrue(Files.exists(distApp));

// when
latch.set(new CountDownLatch(1));
Files.writeString(app, "\nalert(\"foo\");", StandardOpenOption.APPEND);
assertTrue(latch.get().await(2, TimeUnit.SECONDS));
assertNull(bundleException.get(), "No error during bundling");

assertTrue(Files.readString(distApp).contains("alert(\"foo\");"));

// then
watch.stop();
watch.waitForStop();

assertFalse(watch.isAlive());
}

@Test
Expand All @@ -111,22 +127,23 @@ public void shouldWatchWithError() throws URISyntaxException, IOException, Inter
// when
AtomicReference<CountDownLatch> latch = new AtomicReference<>(new CountDownLatch(1));
AtomicReference<BundleException> bundleException = new AtomicReference<>();
final Watch watch = Bundler.watch(options, (error) -> {
error.ifPresent(bundleException::set);
final Watch watch = Bundler.watch(options, (r) -> {
if (!r.isSuccess()) {
bundleException.set(r.bundleException());
}
latch.get().countDown();
}, true);

// then
assertTrue(latch.get().await(2, TimeUnit.SECONDS));
assertNotNull(bundleException.get(), "Error during bundling");
assertTrue(bundleException.get().output().contains("[ERROR] Could not resolve \"\""));
assertTrue(latch.get().getCount() == 1, "First build is not using the listener");
assertTrue(watch.isAlive(), "process is alive");
assertNotNull(watch.firstBuildResult().bundleException(), "Error during bundling");
assertTrue(watch.firstBuildResult().bundleException().output().contains("[ERROR] Could not resolve \"\""));

final Path app = watch.workDir().resolve("application-error.js");
assertTrue(Files.exists(app));

// when
bundleException.set(null);
latch.set(new CountDownLatch(1));
Files.writeString(app, "alert(\"foo\");", StandardOpenOption.TRUNCATE_EXISTING);

// then
Expand Down
Loading