apicollective / apibuilder-generator

MIT License
54 stars 36 forks source link

Generated client swallows error messages for "unsupported" status codes #18

Open haywood opened 9 years ago

haywood commented 9 years ago

In Scala, the generated code swallows the bodies of error responses for "unsupported" codes. Clients are losing valuable debugging information. This can make a simple problem take a long time to figure out, simply because the user can't see if the server is sending a useful error message.

If there is concern about arbitrarily large bodies coming back from the server, we could just embed the first kilobyte of the server's response in the error message. However, hiding it entirely as we do now, produces extremely frustrating error messages.

Example of generated code below.

_executeRequest("GET", s"/agreements", queryParameters = queryParameters).map {
  case r if r.getStatusCode == 200 => _root_.com.gilt.iris.hub.v0.Client.parseJson("Seq[com.gilt.iris.hub.v0.models.Agreement]", r, _.validate[Seq[com.gilt.iris.hub.v0.models.Agreement]])
  case r => throw new com.gilt.iris.hub.v0.errors.FailedRequest(r.getStatusCode, s"Unsupported response code[${r.getStatusCode}]. Expected: 200", requestUri = Some(r.getUri))
}

FailedRequest code below. This class could be further improved by including the requestUri in the message of the exception.


    case class FailedRequest(responseCode: Int, message: String, requestUri: Option[_root_.java.net.URI] = None) extends Exception(s"HTTP $responseCode: $message")
emersonloureiro commented 9 years ago

+1 for this.

haywood commented 9 years ago

Realized that I didn't exactly propose a solution here... I think I want the code to look like the below instead of the above.

_executeRequest("GET", s"/agreements", queryParameters = queryParameters).map {
  case r if r.getStatusCode == 200 => _root_.com.gilt.iris.hub.v0.Client.parseJson("Seq[com.gilt.iris.hub.v0.models.Agreement]", r, _.validate[Seq[com.gilt.iris.hub.v0.models.Agreement]])
  case r => throw new com.gilt.iris.hub.v0.errors.FailedRequest(r.getStatusCode, r.getResponseBody.take(1024), "GET", requestUri = Some(r.getUri))
}
case class FailedRequest(responseCode: Int, message: String, requestMethod: String, requestUri: Option[_root_.java.net.URI] = None) extends Exception(s"${requestMethod} ${requestUri.getOrElse("")} failed, $responseCode: $message")
haywood commented 9 years ago

I will take pass at this when https://github.com/gilt/apidoc-generator/pull/19 gets merged.

mbryzek commented 9 years ago

Proposed changes look great