0

I just write a simple commandwrapper in java, this is construction function:

Process process;
Thread in;
Thread out; 

public CommandWrapper(Process process) {
    this.process = process;
    final InputStream inputStream = process.getInputStream();
    // final BufferedReader
    //final BufferedReader r = new BufferedReader(new InputStreamReader(inputStream));
    final byte[] buffer = new byte[1024];
    out = new Thread() {
        // String line;
        int lineNumber = 0;

        public void run() {
            try {
                while (true) {
                    int count = inputStream.read(buffer);
                    System.out.println(lineNumber + ":"
                            + new String(buffer, 0, count - 1));
                    // line=r.readLine();
                    // System.out.println(lineNumber+":"+line);
                    lineNumber++;
                }
            } catch (Exception e) {

            }
        }
    };
    final BufferedReader reader = new BufferedReader(new InputStreamReader(
            System.in));
    final OutputStream outputStream = process.getOutputStream();
    in = new Thread() {
        String line;

        public void run() {
            try {
                //while (true) {
                    outputStream.write((reader.readLine() + "/n")
                            .getBytes());
                    outputStream.flush();
                //}
            } catch (Exception e) {

            }
        }
    };
}

public void startIn() {
    in.start();
}

This is when it invoke:

public static void main(String[] args) {
    try {
        CommandWrapper command = new CommandWrapper(Runtime.getRuntime()
                .exec("wget www.google.com"));
        //command.startIn();
        command.startOut();
    } catch (Exception e) {
        e.printStackTrace();
    }
}

It works OK when I run simple command like ls -l or other local commander, but when I want to run wget command it is print out nothing as output. I do know why.

Andrew Thompson
  • 168,117
  • 40
  • 217
  • 433
user504909
  • 9,119
  • 12
  • 60
  • 109
  • 2
    Never have an empty try-catch block. At the very least print out the stack trace of the exception. – Diego Basch Dec 26 '12 at 03:01
  • As is common when using the `exec()` command, the above is a very fragile and poorly written example. Be sure to go through (and implement) the tips of the Java World article linked from the [`exec` tag Wiki](http://stackoverflow.com/tags/runtime.exec/info) & the failure reason will probably become more clear. – Andrew Thompson Dec 26 '12 at 03:02
  • I do try-catch at invoke part, do you read the last part of my code? – user504909 Dec 26 '12 at 03:02
  • I believe your comment was to @DiegoBasch (good catch, BTW). While one of the three try/catch statements dumps the stack trace, ***every*** try/catch should do so. Further tips beyond my first comment. 1) Use a `ProcessBuilder` to make the `Process`. Break the `String command` (e.g. `"wget www.google.com"`) into `String[] commands`. – Andrew Thompson Dec 26 '12 at 03:06
  • @user504909 your `run()` methods in `CommandWrapper` don't. Never means **never**. – Diego Basch Dec 26 '12 at 03:23
  • @Diego Basch Thank you. I got it. – user504909 Dec 26 '12 at 03:29
  • Glad you got it sorted. :) You should write up an answer & accept it. – Andrew Thompson Dec 26 '12 at 04:05

1 Answers1

1

From the code you've shown and your description of how you use it, the best guess is that an exception occurs, and you silently swallow it. This happens whenever you have an empty catch-block, like this:

catch (Exception e) {
}

You happen to have one in the run() method of your out thread.

Silently swallowing exceptions is extremely bad practice.

You should never ever ever do this! Depending on your application the appropriate solution varies, but since you're writing a console application you probably want to print the stack trace of the exception. In Java, this is done with e.printStackTrace():

catch (Exception e) {
    e.printStackTrace();
}

Another option (which might not be appropriate in this specific case) is to rethrow the exception, possibly after wrapping it in another exception (for example one you've written specifically for your application):

catch (Exception e) {
    throw e;
}
// or
catch (Exception e) {
    throw new MyOwnException(e);
}

Doing either of these two (printing stack trace or rethrowing) will ensure that no exceptions go unnoticed.

However, no rule without exceptions ;)

There are cases when it is appropriate to have empty catch-clauses. If you know that some operation might throw an exception and you just want to proceed when it happens, an empty catch-clause is a good way to do it. However, the cases where this is appropriated are limited to (at least) the following conditions:

  1. You must know the specific type of the exception. You never want to catch a general exception (i.e. catch (Exception e) since that might be thrown for any reason which you cannot possibly predict. If you use empty catch clauses, always catch specific exception type (such as IOException).

  2. You must know why the exception was thrown. You should only swallow exceptions that you know the origin of. If you swallow any other exceptions, you'll end up like in this situation, where your code doesn't do what you expect and you can't understand why. Swallowed exceptions are extremely difficult to debug, since they are, well, swallowed, and thereby hidden.

  3. You must know that you don't care about the exception. The reason to use empty catch-clauses is mainly (read: only) to handle situations where the code you're using treats something as exceptional, while you do not. By exeptional in this context we mean "something that shouldn't really happen, and if it does, something is seriously wrong."

An example of when empty catch-clauses are appropriate:
Say that you are using someone elses code that opens a file for reading, given the absolute path of the file. Most such routines throw exceptions if the file does not exist - it is the job of the client code (i.e. the code that calls the "open file routine") to ensure that the file exists before trying to open it. Exceptions will also be thrown if, for example, the user running the program does not have permissions to read the file.

Now, you might not really care why the file couldn't be opened, but if it couldn't you just want to keep going - in that case, you swallow all exceptions related to reading the file (in Java, likely an IOException of some sort). Note that you do not swallow all exceptions - only the ones related to opening the file!

Community
  • 1
  • 1
Tomas Aschan
  • 58,548
  • 56
  • 243
  • 402
  • Also, credit is due to [Diego Basch](http://stackoverflow.com/users/586880/diego-basch) for commenting about emtpy catch -clauses and inspiring me to write up this answer. – Tomas Aschan Dec 26 '12 at 03:32