0

So in my android app, which uses GET to pull a webpage and download the text, I perform:

private InputStream OpenHttpConnection(String urlString) throws IOException {
    Log.d("Networking", "InputStream called");
    InputStream in = null;
    int response = -1;

    URL url = new URL(urlString);
    URLConnection conn = url.openConnection();

    if(!(conn instanceof HttpURLConnection))
        throw new IOException("Not an HTTP connection");
    try {
        HttpURLConnection httpConn = (HttpURLConnection) conn;

        httpConn.setConnectTimeout(10000);
        httpConn.setReadTimeout(10000);
        httpConn.setAllowUserInteraction(false);
        httpConn.setInstanceFollowRedirects(true);
        httpConn.setRequestMethod("GET");
        httpConn.connect();
        response = httpConn.getResponseCode();
        if (response == HttpURLConnection.HTTP_OK) {
            in = httpConn.getInputStream();
        }
    }
    catch (Exception ex) {
            Log.d("Networking", "" + ex.getLocalizedMessage());
            throw new IOException("Error connecting");
        }
    return in;
}

private String DownloadText(String URL) {
    int BUFFER_SIZE = 2000;
    InputStream in = null;
    try {
        in = OpenHttpConnection(URL);
        } 
    catch (IOException e) {
        Log.d("Networking", "" + e.getLocalizedMessage());
        return "";
    }
    InputStreamReader isr = new InputStreamReader(in);
    int charRead;
    String str = "";
    char[] inputBuffer = new char[BUFFER_SIZE];
    try {
        while ((charRead = isr.read(inputBuffer))>0) {
            //---convert the chars to a String---
            String readString = String.copyValueOf(inputBuffer, 0, charRead);
            str += readString;
            inputBuffer = new char[BUFFER_SIZE]; }
        in.close();
    }
    catch (IOException e) {
        Log.d("Networking", "" + e.getLocalizedMessage());
        return "";
    }
    return str;
}

This works perfectly if I call stringx = DownloadText("http://hello.com/whatever.txt"), as long as whatever.txt exists.

If it 404s, however, it crashes. This surprises me - a 404 is still returning content, surely? I've put lots of debug in and it seems to execute the line:

    InputStreamReader isr = new InputStreamReader(in);

Before crashing. Nothing after this line executes. I've tried using a try {} catch (IOException) {} around this, but it says the line doesn't throw the exception.

Does anyone have any insight into why this line is causing such a problem? My application is almost complete, but the error handling is causing me problems!

Many thanks!

James
  • 363
  • 1
  • 4
  • 14

1 Answers1

4

This bit of your code:

    if (response == HttpURLConnection.HTTP_OK) {
        in = httpConn.getInputStream();
    }

prevents from in being assigned when you get a 404. If you get anything besides an HTTP_OK (200), in will be null and your InputStreamReader assignment will fail.

To handle this error, you can use the following pattern:

class HTTPException extends IOException {
    private int responseCode;

    public HTTPException( final int responseCode ) {
        super();
        this.responseCode = responseCode;
    }

    public int getResponseCode() {
        return this.responseCode;
    }
}

Then change your code as follows:

private InputStream OpenHttpConnection(String urlString) throws IOException {
    Log.d("Networking", "InputStream called");
    InputStream in = null;
    int response = -1;

    URL url = new URL(urlString);
    URLConnection conn = url.openConnection();

    if(!(conn instanceof HttpURLConnection))
        throw new IOException("Not an HTTP connection");
    try {
        HttpURLConnection httpConn = (HttpURLConnection) conn;

        httpConn.setConnectTimeout(10000);
        httpConn.setReadTimeout(10000);
        httpConn.setAllowUserInteraction(false);
        httpConn.setInstanceFollowRedirects(true);
        httpConn.setRequestMethod("GET");
        httpConn.connect();
        response = httpConn.getResponseCode();
        if (response == HttpURLConnection.HTTP_OK) {
            in = httpConn.getInputStream();
        } else { // this is new
            throw new HTTPException( response );
        }

    }
    catch (Exception ex) {
            Log.d("Networking", "" + ex.getLocalizedMessage());
            throw new IOException("Error connecting");
        }
    return in;
}

private String DownloadText(String URL) {
    int BUFFER_SIZE = 2000;
    InputStream in = null;
    try {
        in = OpenHttpConnection(URL);
        } 
    catch (IOException e) {
        Log.d("Networking", "" + e.getLocalizedMessage());
        return "";
    }
    InputStreamReader isr = new InputStreamReader(in);
    int charRead;
    String str = "";
    char[] inputBuffer = new char[BUFFER_SIZE];
    try {
        while ((charRead = isr.read(inputBuffer))>0) {
            //---convert the chars to a String---
            String readString = String.copyValueOf(inputBuffer, 0, charRead);
            str += readString;
            inputBuffer = new char[BUFFER_SIZE]; }
        in.close();
    }
    catch( HTTPException e ) {
        Log.d( String.format( "HTTP Response not ok: %d", e.getResponseCode() ) );
        // handle whatever else you need to handle here.
    }
    catch (IOException e) {
        Log.d("Networking", "" + e.getLocalizedMessage());
        return "";
    }
    return str;
}
323go
  • 14,143
  • 6
  • 33
  • 41
  • This is so horrendously obvious I am embarrassed, but also very thankful! – James Sep 14 '12 at 13:57
  • How would you recommend I end up returning the fact I've got a 404 then? Lose the if condition and return the 404 error as an input stream, and parse it later on? Or return a value with an: } else return - I'm not really sure how I can return a stream manually, so I'm not sure how to deal with this... – James Sep 14 '12 at 14:00
  • That really depends on how you're calling this. The cleanest way would be to descend your own Exception from IOException and then using a try/catch block in the caller. – 323go Sep 14 '12 at 14:28
  • Updated the Answer. Only had a few minutes, so I didn't do a syntax check. – 323go Sep 14 '12 at 16:28