Starts better bot's process failure management

.gitignore

@@ -3,3 +3,4 @@
 /.project
 /.settings/
 /dependency-reduced-pom.xml
+/data/pgn

src/main/java/com/fathzer/jchess/Game.java

@@ -1,8 +1,10 @@
 package com.fathzer.jchess;
 
+import java.io.IOException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.function.BiConsumer;
+import java.util.function.Consumer;
 
 import com.fathzer.jchess.bot.Engine;
 import com.fathzer.jchess.fen.FENUtils;
@@ -21,35 +23,6 @@ public class Game {
 	    return t;
 	});
 
-	private class EngineTurn implements Runnable {
-		private final Engine engine;
-		private final BiConsumer<Game, Move> moveConsumer;
-
-		private EngineTurn(Engine engine, BiConsumer<Game,Move> moveConsumer) {
-			this.moveConsumer = moveConsumer;
-			this.engine = engine;
-		}
-
-		@Override
-		public void run() {
-			final CoordinatesSystem cs = board.getCoordinatesSystem();
-			engine.setPosition(FENUtils.to(history.getStartBoard()), history.getMoves().stream().map(m -> JChessUCIEngine.toUCIMove(cs, m)).
-					map(UCIMove::toString).toList());
-			final CountDownState params;
-			if (clock==null) {
-				params = null;
-			} else {
-				final long remainingTime = clock.getRemaining(clock.getPlaying());
-				final ClockSettings clockSettings = clock.getCurrentSettings(clock.getPlaying());
-				final int increment = clockSettings.getIncrement()>0 ? clockSettings.getIncrement()/clockSettings.getMovesNumberBeforeIncrement() : 0;
-				final int movesToGo = clock.getRemainingMovesBeforeNext(clock.getPlaying());
-				params = new CountDownState(remainingTime, increment, movesToGo);
-			}
-			Move move = JChessUCIEngine.toMove(board, UCIMove.from(engine.play(params)));
-			moveConsumer.accept(Game.this, move);
-		}
-	}
-
 	@Getter
 	private final Board<Move> board;
 	@Getter
@@ -94,9 +67,32 @@ public void pause() {
 			}
 		}
 	}
+
+	private Move getMove(Engine engine) throws IOException {
+		final CoordinatesSystem cs = board.getCoordinatesSystem();
+		engine.setPosition(FENUtils.to(history.getStartBoard()), history.getMoves().stream().map(m -> JChessUCIEngine.toUCIMove(cs, m)).
+				map(UCIMove::toString).toList());
+		final CountDownState params;
+		if (clock==null) {
+			params = null;
+		} else {
+			final long remainingTime = clock.getRemaining(clock.getPlaying());
+			final ClockSettings clockSettings = clock.getCurrentSettings(clock.getPlaying());
+			final int increment = clockSettings.getIncrement()>0 ? clockSettings.getIncrement()/clockSettings.getMovesNumberBeforeIncrement() : 0;
+			final int movesToGo = clock.getRemainingMovesBeforeNext(clock.getPlaying());
+			params = new CountDownState(remainingTime, increment, movesToGo);
+		}
+		return JChessUCIEngine.toMove(board, UCIMove.from(engine.getMove(params)));
+	}
 
-	public void playEngine(Engine engine, BiConsumer<Game, Move> moveConsumer) {
-		EXECUTOR.execute(new EngineTurn(engine, moveConsumer));
+	public void playEngine(Engine engine, BiConsumer<Game, Move> moveConsumer, Consumer<Exception> errorManager) {
+		EXECUTOR.execute(() -> {
+			try {
+				moveConsumer.accept(this, getMove(engine));
+			} catch (IOException e) {
+				errorManager.accept(e);
+			}
+		});
 	}
 
 	public void onMove(Move move) {

src/main/java/com/fathzer/jchess/bot/Engine.java

@@ -1,12 +1,15 @@
 package com.fathzer.jchess.bot;
 
 import java.io.Closeable;
+import java.io.IOException;
 import java.util.List;
 
 import com.fathzer.games.clock.CountDownState;
 import com.fathzer.jchess.settings.GameSettings.Variant;
 
 public interface Engine extends Closeable {
+	String getName();
+
 	List<Option<?>> getOptions();
 
 	/** Tests whether a variant is supported or not by this engine.
@@ -18,14 +21,16 @@ public interface Engine extends Closeable {
 	/** Starts a new game.
 	 * @param variant The game variant
 	 * @return false if the variant is not supported
+	 * @throws IOException If communication with engine fails
 	 */
-	boolean newGame(Variant variant);
+	boolean newGame(Variant variant) throws IOException;
 
-	void setPosition(String fen, List<String> moves);
+	void setPosition(String fen, List<String> moves) throws IOException;
 
 	/** Gets the engine move choice.
 	 * @param params The current engine's clock count down (null if no clock is set)
 	 * @return a Move in <a href="https://gist.github.com/DOBRO/2592c6dad754ba67e6dcaec8c90165bf">UCI</a> format
+	 * @throws IOException If communication with engine fails
 	 */
-	String play(CountDownState params);
+	String getMove(CountDownState params) throws IOException;
 }

src/main/java/com/fathzer/jchess/bot/uci/EngineLoader.java

@@ -36,7 +36,8 @@ public static void init() throws IOException {
 		if (data!=null) {
 			return;
 		}
-		final EngineData internal = new EngineData("JChess", null, new InternalEngine());
+		final InternalEngine engine = new InternalEngine();
+		final EngineData internal = new EngineData(engine.getName(), null, engine);
 		final EngineData[] array;
 		IOException error = null;
 		if (!Files.exists(PATH)) {

src/main/java/com/fathzer/jchess/bot/uci/UCIEngine.java

@@ -2,6 +2,7 @@
 
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.util.ArrayList;
@@ -17,6 +18,7 @@
 import com.fathzer.jchess.bot.Option.Type;
 import com.fathzer.jchess.bot.uci.EngineLoader.EngineData;
 import com.fathzer.jchess.settings.GameSettings.Variant;
+import com.fathzer.util.ProcessExitDetector;
 
 import lombok.extern.slf4j.Slf4j;
 
@@ -34,15 +36,30 @@ public class UCIEngine implements Engine {
 	private boolean is960Supported;
 	private boolean whiteToPlay;
 	private boolean positionSet;
+	private boolean expectedRunning;
 
 	public UCIEngine(EngineData data) throws IOException {
 		log.info ("Launching process {}", Arrays.asList(data.getCommand()));
 		this.data = data;
 		final ProcessBuilder processBuilder = new ProcessBuilder(data.getCommand());
 		this.process = processBuilder.start();
+		this.expectedRunning = true;
 		this.reader = process.inputReader();
 		this.writer = process.outputWriter();
 		this.errorReader = new StdErrReader(process);
+		new ProcessExitDetector(process, p -> {
+			if (expectedRunning) {
+				expectedRunning = false;
+				log.warn("{} UCI engine exited unexpectedly with code {}", data.getName(), p.exitValue());
+				try {
+					closeStreams();
+				} catch (IOException e) {
+					log.error("The following error occured while closing streams of "+data.getName()+" UCI engine");
+				}
+			} else {
+				log.info("{} UCI engine exited with code {}", data.getName(), p.exitValue());
+			}
+		}).start(true);
 		this.options = new ArrayList<>();
 		init();
 	}
@@ -53,7 +70,7 @@ private void init() throws IOException {
 		do {
 			line = read();
 			if (line==null) {
-				break;
+				throw new EOFException();
 			}
 			final Optional<Option<?>> ooption = parseOption(line);
 			if (ooption.isPresent()) {
@@ -62,7 +79,13 @@ private void init() throws IOException {
 					is960Supported = true;
 				} else if (!PONDER_OPTION.equals(option.getName())) {
 					// Ponder is not supported yet
-					option.addListener((prev, cur) -> setOption(option, cur));
+					option.addListener((prev, cur) -> {
+						try {
+							setOption(option, cur);
+						} catch (IOException e) {
+							throw new UncheckedIOException(e);
+						}
+					});
 					options.add(option);
 				}
 			}
@@ -77,7 +100,7 @@ private Optional<Option<?>> parseOption(String line) throws IOException {
 		}
 	}
 
-	private void setOption(Option<?> option, Object value) {
+	private void setOption(Option<?> option, Object value) throws IOException {
 		final StringBuilder buf = new StringBuilder("setoption name ");
 		buf.append(option.getName());
 		if (Type.BUTTON!=option.getType()) {
@@ -87,24 +110,16 @@ private void setOption(Option<?> option, Object value) {
 		write(buf.toString());
 	}
 
-	private void write(String line) {
-		try {
-			this.writer.write(line);
-			this.writer.newLine();
-			this.writer.flush();
-			log.info(">{}: {}", data.getName(), line);
-		} catch (IOException e) {
-			throw new UncheckedIOException(e);
-		}
+	private void write(String line) throws IOException {
+		this.writer.write(line);
+		this.writer.newLine();
+		this.writer.flush();
+		log.info(">{}: {}", data.getName(), line);
 	}
-	private String read() {
-		try {
-			final String line = reader.readLine();
-			log.info("<{} : {}", data.getName(), line);
-			return line;
-		} catch (IOException e) {
-			throw new UncheckedIOException(e);
-		}
+	private String read() throws IOException {
+		final String line = reader.readLine();
+		log.info("<{} : {}", data.getName(), line);
+		return line;
 	}
 
 	@Override
@@ -118,7 +133,7 @@ public boolean isSupported(Variant variant) {
 	}
 
 	@Override
-	public boolean newGame(Variant variant) {
+	public boolean newGame(Variant variant) throws IOException {
 		positionSet = false;
 		if (variant==Variant.CHESS960 && !is960Supported) {
 			return false;
@@ -135,18 +150,19 @@ public boolean newGame(Variant variant) {
 	 * @param answerValidator a predicate that checks the lines returned by engine. 
 	 * @return The line that is considered valid, null if no valid line is returned
 	 * and the engine closed its output.
+	 * @throws IOException If communication with engine fails
 	 */
-	private String waitAnswer(Predicate<String> answerValidator) {
+	private String waitAnswer(Predicate<String> answerValidator) throws IOException {
 		for (String line = read(); line!=null; line=read()) {
 			if (answerValidator.test(line)) {
 				return line;
 			}
 		}
-		return null;
+		throw new EOFException();
 	}
 
 	@Override
-	public void setPosition(String fen, List<String> moves) {
+	public void setPosition(String fen, List<String> moves) throws IOException {
 		whiteToPlay = "w".equals(fen.split(" ")[1]);
 		if (moves.size()%2!=0) {
 			whiteToPlay = !whiteToPlay;
@@ -164,7 +180,7 @@ public void setPosition(String fen, List<String> moves) {
 	}
 
 	@Override
-	public String play(CountDownState params) {
+	public String getMove(CountDownState params) throws IOException {
 		if (!positionSet) {
 			throw new IllegalStateException("No position defined");
 		}
@@ -190,26 +206,34 @@ public String play(CountDownState params) {
 		write (command.toString());
 		var bestMovePrefix = "bestmove ";
 		final String answer = waitAnswer(s -> s.startsWith(bestMovePrefix));
-		if (answer!=null) {
-			return answer.substring(bestMovePrefix.length());
-		} else {
-			return null;
-		}
+		return answer.substring(bestMovePrefix.length());
 	}
 
 	@Override
 	public void close() throws IOException {
+		if (!expectedRunning) {
+			return;
+		}
+		expectedRunning = false;
 		this.write("quit");
-		this.reader.close();
-		this.writer.close();
-		this.errorReader.close();
+		closeStreams();
 		try {
 			this.process.waitFor(5, TimeUnit.SECONDS);
-			log.info("{} UCI engine exited", data.getName());
 		} catch (InterruptedException e) {
 			log.warn("Fail to gracefully close UCI engine {}, trying to destroy it", data.getName());
 			this.process.destroy();
 			Thread.currentThread().interrupt();
 		}
 	}
+
+	private void closeStreams() throws IOException {
+		this.reader.close();
+		this.writer.close();
+		this.errorReader.close();
+	}
+
+	@Override
+	public String getName() {
+		return data.getName();
+	}
 }

src/main/java/com/fathzer/jchess/internal/InternalEngine.java

@@ -27,6 +27,7 @@
 import com.fathzer.jchess.uci.UCIMove;
 
 public class InternalEngine implements Engine {
+	private static final String NAME = "JChess";
 	private static final long MILLIS_IN_SECONDS = 1000L;
 	private static final BasicTimeManager<Board<Move>> TIME_MANAGER = new BasicTimeManager<>(VuckovicSolakOracle.INSTANCE);
 
@@ -111,7 +112,7 @@ public void setPosition(String startpos, List<String> moves) {
 	}
 
 	@Override
-	public String play(CountDownState countDownState) {
+	public String getMove(CountDownState countDownState) {
 		if (board==null) {
 			throw new IllegalStateException("No position set");
 		}
@@ -130,4 +131,9 @@ public String play(CountDownState countDownState) {
 	public void close() {
 		// Nothing to do
 	}
+
+	@Override
+	public String getName() {
+		return NAME;
+	}
 }

src/main/java/com/fathzer/jchess/swing/GameSession.java

@@ -173,7 +173,10 @@ private void nextMove() {
 		panel.getBoard().setManualMoveEnabled(engine==null);
 		if (engine!=null) {
 			log.debug("Engine detected for {}",activeColor);
-			game.playEngine(engine, this::play);
+			game.playEngine(engine, this::play, e -> {
+				log.error("Error while communicating with "+engine.getName()+" engine", e);
+				SwingUtilities.invokeLater(() -> onEngineError(engine));
+			});
 		}
 	}
 
@@ -246,6 +249,12 @@ private void resign(Color color) {
 			setState(State.RUNNING);
 		}
 	}
+
+	private void onEngineError(Engine engine) {
+		JOptionPane.showMessageDialog(panel, "An error occured while communicating with the "+engine.getName()+" engine. Assuming it resigns", "Error", JOptionPane.ERROR_MESSAGE);
+		final Status status = Color.WHITE.equals(game.getBoard().getActiveColor()) ? Status.BLACK_WON : Status.WHITE_WON;
+		endOfGame(status);
+	}
 
 	private void endOfGame(final Status status) {
 		setState(State.PAUSED);

src/main/java/com/fathzer/jchess/swing/settings/EnginesPanel.java

@@ -172,7 +172,7 @@ private void doStart() {
 			firePropertyChange(STARTED_PROPERTY_NAME, null, engine);
 			refreshLists();
 		} catch (IOException e) {
-			JOptionPane.showMessageDialog(Utils.getOwnerWindow(this), "An exception occurred while startting "+engine.getName()+" engine", "Error", JOptionPane.ERROR_MESSAGE);
+			JOptionPane.showMessageDialog(Utils.getOwnerWindow(this), "An error occurred while starting "+engine.getName()+" engine", "Error", JOptionPane.ERROR_MESSAGE);
 		}
 	}