Skip to content

Commit

Permalink
[SPARK-24660][SHS] Show correct error pages when downloading logs
Browse files Browse the repository at this point in the history
## 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 <[email protected]>

Closes #21644 from mgaido91/SPARK-24660.
  • Loading branch information
mgaido91 authored and Marcelo Vanzin committed Jun 27, 2018
1 parent c04cb2d commit 776befb
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
}

Expand Down
5 changes: 5 additions & 0 deletions core/src/main/scala/org/apache/spark/ui/UIUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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()
}
}

0 comments on commit 776befb

Please sign in to comment.