IOException thrown by the terminal does not get reset on subsequent reads

[hflzh]

Hi @gnodet ,

I tried with 3.7.1 snapshot (with your recent fix for #267), looks like there is an issue with IOException handling in the terminal implementation. Once the terminal throws an IOException, subsequent reads of LineReader#readLine() always hit that IOException, I think subsequent reads should succeed if they do not encounter any IOException. The test codes that reproduce the issue:

System.out.println("=== JLine 3 Test ===");
LineReaderBuilder lrb = LineReaderBuilder.builder();
TerminalBuilder tb = TerminalBuilder.builder();
String[] lines = {
    "test 1",
    "test 2",
    "M",
    "test 3",
    "exit",
};

ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream ps = new PrintStream(baos);
for (String line : lines) {
    ps.println(line);
}
ps.flush();
InputStream bais = new ByteArrayInputStream(baos.toByteArray());

InputStream in = new FilterInputStream(bais) {

    @Override
    public int read() throws IOException {
        int r = in.read();
        if (r == 'M') {
            System.out.println();
            System.out.println("!!! Found 'M', inject an IOException ... !!!");
            System.out.println();
            throw new IOException("Inject IOException");
        }
        return r;
    }
};

Terminal t = tb.system(false).streams(in, System.err).paused(true).build();
System.out.println("=== terminal type: " + t.getType());
System.out.println("=== terminal class: " + t.getClass().getName());
lrb.terminal(t);
LineReader lr = lrb.build();
while (true) {
    if (t.paused()) {
        System.out.println("=== resuming terminal ... ");
        t.resume();
    }
    try {
        String s = lr.readLine("prompt-> ");
        System.out.println("=== readLine() returned: " + s);
        if ("exit".equals(s)) {
            System.out.println("=== about to exit ...");
            break;
        }
    } catch (Throwable ex) {
        ex.printStackTrace();
        System.out.println("=== Caught exception");
    }
}
t.close();
System.out.println("=== done");

Test output:

=== JLine 3 Test ===
=== terminal type: xterm-256color
=== terminal class: org.jline.terminal.impl.ExternalTerminal
=== resuming terminal ... 
test 1
test 2

!!! Found 'M', inject an IOException ... !!!

prompt-> 

java.io.IOError: java.io.IOException: Inject IOException
    at org.jline.keymap.BindingReader.readCharacter(BindingReader.java:142)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:109)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:60)
    at org.jline.reader.impl.LineReaderImpl.readBinding(LineReaderImpl.java:714)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:522)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:385)
    at test.JLine3Test.test6(JLine3Test.java:155)
    at test.JLine3Test.main(JLine3Test.java:25)
Caused by: java.io.IOException: Inject IOException
    at test.JLine3Test$1.read(JLine3Test.java:138)
    at org.jline.terminal.impl.ExternalTerminal.pump(ExternalTerminal.java:121)
    at java.lang.Thread.run(Thread.java:748)
=== Caught exception
prompt-> 

java.io.IOError: java.io.IOException: Inject IOException
    at org.jline.keymap.BindingReader.readCharacter(BindingReader.java:142)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:109)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:60)
    at org.jline.reader.impl.LineReaderImpl.readBinding(LineReaderImpl.java:714)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:522)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:385)
    at test.JLine3Test.test6(JLine3Test.java:155)
    at test.JLine3Test.main(JLine3Test.java:25)
Caused by: java.io.IOException: Inject IOException
    at test.JLine3Test$1.read(JLine3Test.java:138)
    at org.jline.terminal.impl.ExternalTerminal.pump(ExternalTerminal.java:121)
    at java.lang.Thread.run(Thread.java:748)
=== Caught exception
prompt-> 

java.io.IOError: java.io.IOException: Inject IOException
    at org.jline.keymap.BindingReader.readCharacter(BindingReader.java:142)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:109)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:60)
    at org.jline.reader.impl.LineReaderImpl.readBinding(LineReaderImpl.java:714)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:522)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:385)
    at test.JLine3Test.test6(JLine3Test.java:155)
    at test.JLine3Test.main(JLine3Test.java:25)
Caused by: java.io.IOException: Inject IOException
    at test.JLine3Test$1.read(JLine3Test.java:138)
    at org.jline.terminal.impl.ExternalTerminal.pump(ExternalTerminal.java:121)
    at java.lang.Thread.run(Thread.java:748)
=== Caught exception
prompt-> 

java.io.IOError: java.io.IOException: Inject IOException
    at org.jline.keymap.BindingReader.readCharacter(BindingReader.java:142)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:109)
    at org.jline.keymap.BindingReader.readBinding(BindingReader.java:60)
    at org.jline.reader.impl.LineReaderImpl.readBinding(LineReaderImpl.java:714)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:522)
    at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:385)
    at test.JLine3Test.test6(JLine3Test.java:155)
    at test.JLine3Test.main(JLine3Test.java:25)
Caused by: java.io.IOException: Inject IOException
    at test.JLine3Test$1.read(JLine3Test.java:138)
    at org.jline.terminal.impl.ExternalTerminal.pump(ExternalTerminal.java:121)
    at java.lang.Thread.run(Thread.java:748)
=== Caught exception

[hflzh]

Hi @gnodet , I tried with the fix in 28e36be, LineReader.readLine() was not able to proceed because of EndOfFileException, the lines following the 'M' were never read and printed out.

[hflzh]

I finally figured it out, I have to call terminal.resume() in the catch block to create a new pump thread to read in the inputs.

[hflzh]

Hi @gnodet ,

If I move 'M' to the first line, that is

String[] lines = {
    "M",
    "test 1",
    "test 2",
    "test 3",
    "exit",
};

in this case, the first read attempt would run into IOException, which is expected. However, subsequent reads fail with EndOfFileException, I think subsequent reads should return test 1 , test 2, test 3, exit, can you please take a look if this is an issue?

[gnodet]

I'm not sure we want to go this way. The reason is that usually, streams exceptions are not recoverable, so we'll run a loop throwing an exception forever. This happens here: https://github.com/jline/jline3/blob/master/terminal/src/main/java/org/jline/terminal/impl/ExternalTerminal.java#L119-L141

When an exception is thrown by the underlying stream, the loop exits. If we catch the exception inside the loop, the loop will run forever, usually doing nothing but throwing / catching an exception. A good example is a socket stream : if the socket has been disconnected, you can't really recover from it. So maybe the current implementation is wrong, and the exception should be thrown after all the bytes already read have been consumed, but then always throw the exception and not reset it or throw an EOF.

[hflzh]

the exception should be thrown after all the bytes already read have been consumed, but then always throw the exception and not reset it or throw an EOF.

I think in this case, the exact exception (be it IOException or any other exceptions) that the underlying stream encounters , rather than EOF, should be thrown to users over the LineReader.readLine() API, it is up to the user to decide whether to retry reads upon receiving the exceptions, or stop reading.

As regard to reset exceptions, in my opinion, we could reset the exception. If we do not reset the exception, the user would hit the same exception repeatedly, he is not able to retry reads no matter the underlying stream is recoverable or not.

[gnodet]

The problem is that the pump mechanism does not depend on the user, so it's not up to the user. do you have a use case where an InputStream can be correctly recovered after an IO exception ?

[hflzh]

I do not have a real use case, the test above is to test out how jline API behaves in the scenario. I think some IOExceptions such as SocketTimeoutException is transient and a retry read could succeed.

[gnodet]

Do you think you could come up with a PR ?

[hflzh]

ok, I'll look at the issue and try to create a PR.