Repository: spark
Updated Branches:
  refs/heads/master c04cb2d1b -> 776befbfd


[SPARK-24660][SHS] Show correct error pages when downloading logs

## What changes were proposed in this pull request?

SHS is showing bad errors when trying to download logs is not successful. This 
may happen because the requested application doesn't exist or the user doesn't 
have permissions for it, for instance.

The PR fixes the response when errors occur, so that they are displayed 
properly.

## How was this patch tested?

manual tests

**Before the patch:**
 1. Unauthorized user
![screen shot 2018-06-26 at 3 53 33 
pm](https://user-images.githubusercontent.com/8821783/41918118-f8b37e70-795b-11e8-91e8-d0250239f09d.png)

 2. Non-existing application
![screen shot 2018-06-26 at 3 25 19 
pm](https://user-images.githubusercontent.com/8821783/41918082-e3034c72-795b-11e8-970e-cee4a1eae77f.png)

**After the patch**
 1. Unauthorized user
![screen shot 2018-06-26 at 3 41 29 
pm](https://user-images.githubusercontent.com/8821783/41918155-0d950476-795c-11e8-8d26-7b7ce73e6fe1.png)

 2. Non-existing application
![screen shot 2018-06-26 at 3 40 37 
pm](https://user-images.githubusercontent.com/8821783/41918175-1a14bb88-795c-11e8-91ab-eadf29190a02.png)

Author: Marco Gaido <marcogaid...@gmail.com>

Closes #21644 from mgaido91/SPARK-24660.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/776befbf
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/776befbf
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/776befbf

Branch: refs/heads/master
Commit: 776befbfd5b3c317a713d4fa3882cda6264db9ba
Parents: c04cb2d
Author: Marco Gaido <marcogaid...@gmail.com>
Authored: Wed Jun 27 14:26:08 2018 -0700
Committer: Marcelo Vanzin <van...@cloudera.com>
Committed: Wed Jun 27 14:26:08 2018 -0700

----------------------------------------------------------------------
 .../spark/status/api/v1/ApiRootResource.scala   | 30 ++++----------------
 .../status/api/v1/JacksonMessageWriter.scala    |  5 +---
 .../status/api/v1/OneApplicationResource.scala  |  7 ++---
 .../scala/org/apache/spark/ui/UIUtils.scala     |  5 ++++
 4 files changed, 13 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/776befbf/core/src/main/scala/org/apache/spark/status/api/v1/ApiRootResource.scala
----------------------------------------------------------------------
diff --git 
a/core/src/main/scala/org/apache/spark/status/api/v1/ApiRootResource.scala 
b/core/src/main/scala/org/apache/spark/status/api/v1/ApiRootResource.scala
index d121068..84c2ad4 100644
--- a/core/src/main/scala/org/apache/spark/status/api/v1/ApiRootResource.scala
+++ b/core/src/main/scala/org/apache/spark/status/api/v1/ApiRootResource.scala
@@ -28,7 +28,7 @@ import org.glassfish.jersey.server.ServerProperties
 import org.glassfish.jersey.servlet.ServletContainer
 
 import org.apache.spark.SecurityManager
-import org.apache.spark.ui.SparkUI
+import org.apache.spark.ui.{SparkUI, UIUtils}
 
 /**
  * Main entry point for serving spark application metrics as json, using 
JAX-RS.
@@ -148,38 +148,18 @@ private[v1] trait BaseAppResource extends 
ApiRequestContext {
 }
 
 private[v1] class ForbiddenException(msg: String) extends 
WebApplicationException(
-  Response.status(Response.Status.FORBIDDEN).entity(msg).build())
+    UIUtils.buildErrorResponse(Response.Status.FORBIDDEN, msg))
 
 private[v1] class NotFoundException(msg: String) extends 
WebApplicationException(
-  new NoSuchElementException(msg),
-    Response
-      .status(Response.Status.NOT_FOUND)
-      .entity(ErrorWrapper(msg))
-      .build()
-)
+    UIUtils.buildErrorResponse(Response.Status.NOT_FOUND, msg))
 
 private[v1] class ServiceUnavailable(msg: String) extends 
WebApplicationException(
-  new ServiceUnavailableException(msg),
-  Response
-    .status(Response.Status.SERVICE_UNAVAILABLE)
-    .entity(ErrorWrapper(msg))
-    .build()
-)
+    UIUtils.buildErrorResponse(Response.Status.SERVICE_UNAVAILABLE, msg))
 
 private[v1] class BadParameterException(msg: String) extends 
WebApplicationException(
-  new IllegalArgumentException(msg),
-  Response
-    .status(Response.Status.BAD_REQUEST)
-    .entity(ErrorWrapper(msg))
-    .build()
-) {
+    UIUtils.buildErrorResponse(Response.Status.BAD_REQUEST, msg)) {
   def this(param: String, exp: String, actual: String) = {
     this(raw"""Bad value for parameter "$param".  Expected a $exp, got 
"$actual"""")
   }
 }
 
-/**
- * Signal to JacksonMessageWriter to not convert the message into json (which 
would result in an
- * extra set of quotes).
- */
-private[v1] case class ErrorWrapper(s: String)

http://git-wip-us.apache.org/repos/asf/spark/blob/776befbf/core/src/main/scala/org/apache/spark/status/api/v1/JacksonMessageWriter.scala
----------------------------------------------------------------------
diff --git 
a/core/src/main/scala/org/apache/spark/status/api/v1/JacksonMessageWriter.scala 
b/core/src/main/scala/org/apache/spark/status/api/v1/JacksonMessageWriter.scala
index 76af33c..4560d30 100644
--- 
a/core/src/main/scala/org/apache/spark/status/api/v1/JacksonMessageWriter.scala
+++ 
b/core/src/main/scala/org/apache/spark/status/api/v1/JacksonMessageWriter.scala
@@ -68,10 +68,7 @@ private[v1] class JacksonMessageWriter extends 
MessageBodyWriter[Object]{
       mediaType: MediaType,
       multivaluedMap: MultivaluedMap[String, AnyRef],
       outputStream: OutputStream): Unit = {
-    t match {
-      case ErrorWrapper(err) => 
outputStream.write(err.getBytes(StandardCharsets.UTF_8))
-      case _ => mapper.writeValue(outputStream, t)
-    }
+    mapper.writeValue(outputStream, t)
   }
 
   override def getSize(

http://git-wip-us.apache.org/repos/asf/spark/blob/776befbf/core/src/main/scala/org/apache/spark/status/api/v1/OneApplicationResource.scala
----------------------------------------------------------------------
diff --git 
a/core/src/main/scala/org/apache/spark/status/api/v1/OneApplicationResource.scala
 
b/core/src/main/scala/org/apache/spark/status/api/v1/OneApplicationResource.scala
index 9746978..32100c5 100644
--- 
a/core/src/main/scala/org/apache/spark/status/api/v1/OneApplicationResource.scala
+++ 
b/core/src/main/scala/org/apache/spark/status/api/v1/OneApplicationResource.scala
@@ -140,11 +140,8 @@ private[v1] class AbstractApplicationResource extends 
BaseAppResource {
         .header("Content-Type", MediaType.APPLICATION_OCTET_STREAM)
         .build()
     } catch {
-      case NonFatal(e) =>
-        Response.serverError()
-          .entity(s"Event logs are not available for app: $appId.")
-          .status(Response.Status.SERVICE_UNAVAILABLE)
-          .build()
+      case NonFatal(_) =>
+        throw new ServiceUnavailable(s"Event logs are not available for app: 
$appId.")
     }
   }
 

http://git-wip-us.apache.org/repos/asf/spark/blob/776befbf/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala 
b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
index 5d015b0..732b752 100644
--- a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -21,6 +21,7 @@ import java.net.URLDecoder
 import java.text.SimpleDateFormat
 import java.util.{Date, Locale, TimeZone}
 import javax.servlet.http.HttpServletRequest
+import javax.ws.rs.core.{MediaType, Response}
 
 import scala.util.control.NonFatal
 import scala.xml._
@@ -566,4 +567,8 @@ private[spark] object UIUtils extends Logging {
         NEWLINE_AND_SINGLE_QUOTE_REGEX.replaceAllIn(requestParameter, ""))
     }
   }
+
+  def buildErrorResponse(status: Response.Status, msg: String): Response = {
+    Response.status(status).entity(msg).`type`(MediaType.TEXT_PLAIN).build()
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to