Simplify the interface of URL.decode by only returning one possible…

… `Exception`, instead of all possible `Exception` (#3017)
zio · Aug 16, 2024 · 3d1ef0c · 3d1ef0c
1 parent 98b95b2
commit 3d1ef0c
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 15 deletions.
diff --git a/zio-http/shared/src/main/scala-2/zio/http/UrlInterpolator.scala b/zio-http/shared/src/main/scala-2/zio/http/UrlInterpolator.scala
@@ -32,7 +32,7 @@ private[http] object UrlInterpolatorMacro {
     c.prefix.tree match {
       case Apply(_, List(Apply(_, Literal(Constant(p: String)) :: Nil))) =>
         val result = URL.decode(p) match {
-          case Left(error) => c.abort(c.enclosingPosition, s"Invalid URL: ${error.getMessage}")
+          case Left(error) => c.abort(c.enclosingPosition, error.getMessage)
           case Right(url)  =>
             val uri = url.encode
             q"_root_.zio.http.URL.fromURI(new _root_.java.net.URI($uri)).get"
@@ -68,9 +68,8 @@ private[http] object UrlInterpolatorMacro {
         val exampleParts = staticParts.zipAll(injectedPartExamples, "", "").flatMap { case (a, b) => List(a, b) }
         val example      = exampleParts.mkString
         URL.decode(example) match {
-          case Left(error) =>
-            c.abort(c.enclosingPosition, s"Invalid URL: ${error.getMessage}")
-          case Right(url)  =>
+          case Left(error) => c.abort(c.enclosingPosition, error.getMessage)
+          case Right(_)    =>
             val parts =
               staticParts.map { s => Literal(Constant(s)) }
                 .zipAll(args.map(_.tree), Literal(Constant("")), Literal(Constant("")))

diff --git a/zio-http/shared/src/main/scala/zio/http/ConnectionPoolConfig.scala b/zio-http/shared/src/main/scala/zio/http/ConnectionPoolConfig.scala
@@ -47,7 +47,7 @@ object ConnectionPoolConfig {
             URL
               .decode(s)
               .left
-              .map(error => Config.Error.InvalidData(message = s"Invalid URL: ${error.getMessage}"))
+              .map(error => Config.Error.InvalidData(message = error.getMessage))
               .flatMap { url =>
                 url.kind match {
                   case url: URL.Location.Absolute => Right(url -> fixed)
@@ -67,7 +67,7 @@ object ConnectionPoolConfig {
             URL
               .decode(s)
               .left
-              .map(error => Config.Error.InvalidData(message = s"Invalid URL: ${error.getMessage}"))
+              .map(error => Config.Error.InvalidData(message = error.getMessage))
               .flatMap { url =>
                 url.kind match {
                   case url: URL.Location.Absolute => Right(url -> fixed)

diff --git a/zio-http/shared/src/main/scala/zio/http/Request.scala b/zio-http/shared/src/main/scala/zio/http/Request.scala
@@ -223,7 +223,7 @@ object Request {
    */
   private def pathOrUrl(path: String): URL =
     if (path.startsWith("http://") || path.startsWith("https://")) {
-      URL.decode(path).toOption.getOrElse(URL(Path(path)))
+      URL.decode(path).getOrElse(URL(Path(path)))
     } else {
       URL(Path(path))
     }

diff --git a/zio-http/shared/src/main/scala/zio/http/URL.scala b/zio-http/shared/src/main/scala/zio/http/URL.scala
@@ -18,6 +18,8 @@ package zio.http
 
 import java.net.{MalformedURLException, URI}
 
+import scala.util.control.NonFatal
+
 import zio.Config
 
 import zio.http.URL.{Fragment, Location}
@@ -279,26 +281,34 @@ final case class URL(
 }
 
 object URL {
-  def empty: URL = URL(Path.empty)
+  val empty: URL = URL(path = Path.empty)
+
+  /**
+   * To better understand this implementation, read discussion:
+   * https://github.com/zio/zio-http/pull/3017/files#r1716489733
+   */
+  private final class Err(rawUrl: String, cause: Throwable) extends MalformedURLException {
+    override def getMessage: String  = s"""Invalid URL: "$rawUrl""""
+    override def getCause: Throwable = cause
+  }
 
-  def decode(string: String): Either[Exception, URL] = {
-    def invalidURL(string: String) = Left(new MalformedURLException(s"""Invalid URL: "$string""""))
+  def decode(rawUrl: String): Either[MalformedURLException, URL] = {
+    def invalidURL(e: Throwable = null): Either[MalformedURLException, URL] = Left(new Err(rawUrl = rawUrl, cause = e))
 
     try {
-      val uri = new URI(string)
+      val uri = new URI(rawUrl)
       val url = if (uri.isAbsolute) fromAbsoluteURI(uri) else fromRelativeURI(uri)
 
       url match {
-        case None        => invalidURL(string)
         case Some(value) => Right(value)
+        case None        => invalidURL()
       }
-
     } catch {
-      case e: Exception => Left(e)
+      case NonFatal(e) => invalidURL(e)
     }
   }
 
-  def config: Config[URL] = Config.string.mapAttempt(decode(_).toTry.get)
+  def config: Config[URL] = Config.string.mapAttempt(decode(_).fold(throw _, identity))
 
   def fromURI(uri: URI): Option[URL] = if (uri.isAbsolute) fromAbsoluteURI(uri) else fromRelativeURI(uri)