From f5a9833f3f28f256bbd8878fd7da8ac7e268d1a9 Mon Sep 17 00:00:00 2001
From: Andrew Or <andrew@databricks.com>
Date: Mon, 1 Jun 2015 19:39:03 -0700
Subject: [PATCH] [MINOR] [UI] Improve error message on log page

Currently if a bad log type if specified, then we get blank.
We should provide a more informative error message.
---
 .../spark/deploy/worker/ui/LogPage.scala      |  6 ++
 .../spark/deploy/worker/ui/LogPageSuite.scala | 68 +++++++++++++++++++
 2 files changed, 74 insertions(+)
 create mode 100644 core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala

diff --git a/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala b/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
index 88170d4df3..dc2bee6f2b 100644
--- a/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
@@ -29,6 +29,7 @@ import org.apache.spark.util.logging.RollingFileAppender
 private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with Logging {
   private val worker = parent.worker
   private val workDir = parent.workDir
+  private val supportedLogTypes = Set("stderr", "stdout")
 
   def renderLog(request: HttpServletRequest): String = {
     val defaultBytes = 100 * 1024
@@ -129,6 +130,11 @@ private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with
       offsetOption: Option[Long],
       byteLength: Int
     ): (String, Long, Long, Long) = {
+
+    if (!supportedLogTypes.contains(logType)) {
+      return ("Error: Log type must be one of " + supportedLogTypes.mkString(", "), 0, 0, 0)
+    }
+
     try {
       val files = RollingFileAppender.getSortedRolledOverFiles(logDirectory, logType)
       logDebug(s"Sorted log files of type $logType in $logDirectory:\n${files.mkString("\n")}")
diff --git a/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala b/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala
new file mode 100644
index 0000000000..2d02122403
--- /dev/null
+++ b/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy.worker.ui
+
+import java.io.{File, FileWriter}
+
+import org.mockito.Mockito.mock
+import org.scalatest.{FunSuite, PrivateMethodTester}
+
+class LogPageSuite extends FunSuite with PrivateMethodTester {
+
+  test("get logs simple") {
+    val webui = mock(classOf[WorkerWebUI])
+    val logPage = new LogPage(webui)
+
+    // Prepare some fake log files to read later
+    val out = "some stdout here"
+    val err = "some stderr here"
+    val tmpDir = new File(sys.props("java.io.tmpdir"))
+    val tmpOut = new File(tmpDir, "stdout")
+    val tmpErr = new File(tmpDir, "stderr")
+    val tmpRand = new File(tmpDir, "random")
+    write(tmpOut, out)
+    write(tmpErr, err)
+    write(tmpRand, "1 6 4 5 2 7 8")
+
+    // Get the logs. All log types other than "stderr" or "stdout" will be rejected
+    val getLog = PrivateMethod[(String, Long, Long, Long)]('getLog)
+    val (stdout, _, _, _) =
+      logPage invokePrivate getLog(tmpDir.getAbsolutePath, "stdout", None, 100)
+    val (stderr, _, _, _) =
+      logPage invokePrivate getLog(tmpDir.getAbsolutePath, "stderr", None, 100)
+    val (error1, _, _, _) =
+      logPage invokePrivate getLog(tmpDir.getAbsolutePath, "random", None, 100)
+    val (error2, _, _, _) =
+      logPage invokePrivate getLog(tmpDir.getAbsolutePath, "does-not-exist.txt", None, 100)
+    assert(stdout === out)
+    assert(stderr === err)
+    assert(error1.startsWith("Error"))
+    assert(error2.startsWith("Error"))
+  }
+
+  /** Write the specified string to the file. */
+  private def write(f: File, s: String): Unit = {
+    val writer = new FileWriter(f)
+    try {
+      writer.write(s)
+    } finally {
+      writer.close()
+    }
+  }
+
+}