Better exception messages

[BenFradet]

I assumed the url param to toEntity was there for some purpose.

We can also go the other way around and remove it altogether and not have the url in the exception message.

[fedefernandez]

Hi @BenFradet, thanks for the PR. I'm wondering if you could create a branch in the 47deg/github4s repo for this PR or the next ones, just to see if the tests pass in Travis. If you don't have permissions to do that, please let me know.

The changes look good to me, but I think we could add a couple of fields to the JsonParsingException and UnexpectedException exceptions for the status code and the headers, because it would be useful for recovery mechanisms. Feel free to add it to this PR or create an issue.

[BenFradet]

I'll try your suggestions on 47deg/github4s 👍

[BenFradet]

@fedefernandez Doesn't work with js though: https://github.com/47deg/github4s/blob/master/github4s/js/src/main/scala/github4s/HttpRequestBuilderExtensionJS.scala#L65.

Do you propose introducing a new exception type (HttpException / JsonParsingException / UnexpectedException) or having default (which would be cumbersome imo)?

[fedefernandez]

I see, I was thinking in something like:

  case class HttpResponse(statusCode: Int, headers: Map[String, IndexedSeq[String]])

  case class GHResult[A](result: A, httpResponse: HttpResponse)

  sealed abstract class GHException(
      msg: String,
      cause: Option[Throwable] = None,
      httpResponse: Option[HttpResponse] = None)
      extends Throwable(msg) {
    cause foreach initCause
  }

But I think it's out of the scope of the original idea of this PR, we could tackle it in another PR.

[BenFradet]

yes, ok, I'll give it more thoughts