Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 6 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8b26a53
[SPARK-24660][SHS] Show correct error pages when downloading logs
mgaido91 Jun 26, 2018
9934d47
refactor adding one method to UIUtils
mgaido91 Jun 27, 2018
43efe0f
fix scalastyle
mgaido91 Jun 27, 2018
0be2525
fix
mgaido91 Jun 27, 2018
a9baa7c
remove redundant exception
mgaido91 Jun 27, 2018
0c6bbbc
address comment
mgaido91 Jun 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions core/src/main/scala/org/apache/spark/status/api/v1/ApiRootResource.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import java.util.zip.ZipOutputStream
import javax.servlet.ServletContext
import javax.servlet.http.HttpServletRequest
import javax.ws.rs._
import javax.ws.rs.core.{Context, Response}
import javax.ws.rs.core.{Context, MediaType, Response}

import org.eclipse.jetty.server.handler.ContextHandler
import org.eclipse.jetty.servlet.{ServletContextHandler, ServletHolder}
Expand Down Expand Up @@ -148,38 +148,36 @@ private[v1] trait BaseAppResource extends ApiRequestContext {
}

private[v1] class ForbiddenException(msg: String) extends WebApplicationException(
Response.status(Response.Status.FORBIDDEN).entity(msg).build())
Response.status(Response.Status.FORBIDDEN).entity(msg).`type`(MediaType.TEXT_PLAIN).build())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can extract a new helper function to build the response object from a Response.Status and a message. I think UIUtils is good place for such a function then you can use it at OneApplicationResource.scala too as the serverErrormethod just sets the status to Status.INTERNAL_SERVER_ERROR which overwritten by right away with Response.Status.SERVICE_UNAVAILABLE.


private[v1] class NotFoundException(msg: String) extends WebApplicationException(
new NoSuchElementException(msg),
Response
.status(Response.Status.NOT_FOUND)
.entity(ErrorWrapper(msg))
.entity(msg)
.`type`(MediaType.TEXT_PLAIN)
.build()
)

private[v1] class ServiceUnavailable(msg: String) extends WebApplicationException(
new ServiceUnavailableException(msg),
Response
.status(Response.Status.SERVICE_UNAVAILABLE)
.entity(ErrorWrapper(msg))
.entity(msg)
.`type`(MediaType.TEXT_PLAIN)
.build()
)

private[v1] class BadParameterException(msg: String) extends WebApplicationException(
new IllegalArgumentException(msg),
Response
.status(Response.Status.BAD_REQUEST)
.entity(ErrorWrapper(msg))
.entity(msg)
.`type`(MediaType.TEXT_PLAIN)
.build()
) {
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)
5 changes: 1 addition & 4 deletions core/src/main/scala/org/apache/spark/status/api/v1/JacksonMessageWriter.scala
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion core/src/main/scala/org/apache/spark/status/api/v1/OneApplicationResource.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,10 @@ private[v1] class AbstractApplicationResource extends BaseAppResource {
.header("Content-Type", MediaType.APPLICATION_OCTET_STREAM)
.build()
} catch {
case NonFatal(e) =>
case NonFatal(_) =>
Response.serverError()
.entity(s"Event logs are not available for app: $appId.")
.`type`(MediaType.TEXT_PLAIN)
.status(Response.Status.SERVICE_UNAVAILABLE)
.build()
}
Expand Down