Thursday, 27 June 2013

Missing POST body in Android HttpsURLConnection request

This was driving me nuts. I had an app calling a web service. It all worked fine. I added a new web service method and changed my app to call the new method. All of a sudden, nothing worked any more.
I knew the web service was OK because I could call it from a test harness. I knew I hadn't changed any code and I knew that it used to work fine. The failure was really early on in the web service so I knew the web service was being called but that it couldn't find any post variables and returned an error (I returned an HTTP 409 so I knew it was definitely coming from there).
Basically, the HttpsURLConnection which is a subclass of HttpURLConnection is a funny beast. I can see why people still swear by the Apache web client despite Android "advising" that people use the Java class.
So my first piece of code was this:

conn = (HttpsURLConnection)url2.openConnection();
conn.setRequestProperty("Accept-Charset", CHARSET);
conn.setRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=" + CHARSET);

Now the first weird thing is that calling openConnection does NOT actually open a connection, it merely starts the process. I know this because if you call setDoOutput() on a connected connection, it throws an IllegalStateException and this was happening to me. It is daft to call it openConnection if it doesn't. They should deprecate this method and call it createConnection().

Anyway, the exception meant that the connection was open. How could it be if I know that openConnection does not connect and the immediate next line throws an exception? After fannying with the debugger and trying to attach source, surely enough, conn.connected in the debugger window showed false. It was not connected (as expected) bu setDoOutput effectively just says, if ( connected) throw IllegalStateException.

I tried to attach a proxy to see at what point a connection was established but the error occurs too soon to see what was happening. Also, in my code, I was catching the error and carrying on when I should have been returning an error, which caused another error further on.

Eventually, it dawned on me. There was no proxy stripping out my POST body, I had added conn.getResponseCode() to the DEBUGGER watch window. Naturally, this is evaluated after every line of code is stepped over. What it does under the covers, is opens the connection if not already open and asks for a response code. Effectively, this was before my POST body was setup and unsurprisingly, this caused the error 409 to occur. Removing this sorted out the problem.

Why would I do this? Well, another (in my opinion) stupid design in this class is that if the server returns any 4XX code, rather than just report this as a 4XX and have the error text in the response stream, getInputStream throws an exception! A FileNotFoundException of all things (really, I have not made this up!). To avoid this, you have to check for the response code not being 4XX before you attempt to open the input stream. I was doing this and having problems with the codes which is why I had added it to the watch window. From then on, everything had gone down hill.

This is the law of unforeseen circumstances. A poor design has cost me time/money and although the Eclipse debugger was behaving as expected, the strangeness of HttpURLConnection had caused me all kinds of problems.

So, here are my list of major weirdness design issues in the classes:

  1. url.openConnection does not open a connection and should not be called that, it is misleading.
  2. You should NOT have to call setDoInput and setDoOutput just to tell the connection whether you are writing to or reading from the request/response. This is all normal behaviour and should be transparent.
  3. Input and output are confusing because output actually refers to the "output to the other end" and not the response, which is what I think most people would expect. Why not call then requestBody and responseBody?
  4. Calling getInputStream should definitely NOT throw an exception. A 4XX error is not necessarily exceptional and in many cases, there will be data in the response with more information that the caller would want to interrogate.
I if knew enough about Java, I would write my own version of these classes, but reading other people's rants makes me think that Oracle should just rewrite it and move them to another namespace to avoid any confusion. Microsoft do this in .net to their credit when they realise that something was written wrongly.
Post a Comment