clj-http
clj-http copied to clipboard
dakrone
Addressing a problem with 404 errors for EDN requests
Regardless of the requested content type, if an error occurs (such as 404) then the server will often return HTML or plain text to indicate the error. If EDN is requested and status code exceptions turned off, then the returned data is parsed as EDN, leading to a parse exception, and the status is lost.
This patch allows successful EDN requests to go through standard parsing with no change. Unsuccessful requests will attempt to parse EDN (just in case the server really is trying to return a data structure... something I've yet to encounter), but if an error is encountered, it returns the body as raw text.
Is this how other HTTP clients handle this kind of error? It seems like a bug in a server if it doesn't return the correct content-type. There's certainly a good argument that errors should be caught and thrown in such a way that the developer can recover the response, but I'm not so sure about just returning a plain string when the developer has asked for and is expecting EDN.
It's only a bug in the server if the status code is in the 2xx range. If it's in the 4xx range then the body is undefined. It isn't nice to get a content-type with a 404 though.
Thinking on it further, returning the body as a string was incorrect here, since it could be anything. I'm seeing an HTML page saying "Not found" which is why I handled it as a string, but there are no guarantees, so I realize that this was not correct. Returning the byte array would better, allowing the user to work with whatever their server is giving them.
I think we were talking about different content-types: you were talking about the one that the client sends, and I was talking about the one returned by the server?
The issue seems to be that if you request an EDN content type, but the server returns an error with the content type of text/plain or HTML then clj-http still tries to parse it as Clojure? Is that the case?
Hmm... I think rather than the "try and then fall back" approach, I'd rather have a boolean value that allows you to specify whether to decode only on success, or all the time. Then it can be used for all the coercions (not just the EDN one).
What do you think @quoll & @danielcompton ?
@quoll I’m still not clear on the exact flow that is leading to your error, can you post the headers and body of the HTTP request and response?