From 48a92df719e11fd52de46911ed294eeffe926f80 Mon Sep 17 00:00:00 2001 From: KOUNOIKE Yuusuke Date: Mon, 14 Aug 2017 18:36:34 +0900 Subject: [PATCH 1/2] Handle errors to show errors prettify and logging it. --- .../core/controller/ControllerBase.scala | 17 +++++++++++++++++ src/main/twirl/gitbucket/core/error.scala.html | 14 ++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/ControllerBase.scala b/src/main/scala/gitbucket/core/controller/ControllerBase.scala index 60e7893c9..41b45cd11 100644 --- a/src/main/scala/gitbucket/core/controller/ControllerBase.scala +++ b/src/main/scala/gitbucket/core/controller/ControllerBase.scala @@ -26,6 +26,7 @@ import org.eclipse.jgit.lib.ObjectId import org.eclipse.jgit.revwalk.RevCommit import org.eclipse.jgit.treewalk._ import org.apache.commons.io.IOUtils +import org.slf4j.LoggerFactory /** * Provides generic features for controller implementations. @@ -34,6 +35,8 @@ abstract class ControllerBase extends ScalatraFilter with ClientSideValidationFormSupport with JacksonJsonSupport with I18nSupport with FlashMapSupport with Validations with SystemSettingsService { + private val logger = LoggerFactory.getLogger(getClass) + implicit val jsonFormats = gitbucket.core.api.JsonFormat.jsonFormats before("/api/v3/*") { @@ -147,6 +150,20 @@ abstract class ControllerBase extends ScalatraFilter } } + error{ + case e => { + logger.error(s"Catch unhandled error in request: ${request}", e) + if(request.hasAttribute(Keys.Request.Ajax)){ + org.scalatra.InternalServerError() + } else if(request.hasAttribute(Keys.Request.APIv3)){ + contentType = formats("json") + org.scalatra.InternalServerError(ApiError("Internal Server Error")) + } else { + org.scalatra.InternalServerError(gitbucket.core.html.error("Internal Server Error", Some(e))) + } + } + } + override def url(path: String, params: Iterable[(String, Any)] = Iterable.empty, includeContextPath: Boolean = true, includeServletPath: Boolean = true, absolutize: Boolean = true, withSessionId: Boolean = true) diff --git a/src/main/twirl/gitbucket/core/error.scala.html b/src/main/twirl/gitbucket/core/error.scala.html index 3c987d831..211073cbc 100644 --- a/src/main/twirl/gitbucket/core/error.scala.html +++ b/src/main/twirl/gitbucket/core/error.scala.html @@ -1,8 +1,18 @@ -@(title: String)(implicit context: gitbucket.core.controller.Context) +@(title: String, e: Option[Throwable]=None)(implicit context: gitbucket.core.controller.Context) @gitbucket.core.html.main("Error"){

@title

+ @e.map { ex => +

@ex.getMessage

+ + + @ex.getStackTrace.map{ st => + + } + +
@st
+ }
-} \ No newline at end of file +} From 040d812f2a862b48165187a61db2839d0909af58 Mon Sep 17 00:00:00 2001 From: KOUNOIKE Yuusuke Date: Thu, 17 Aug 2017 17:48:53 +0900 Subject: [PATCH 2/2] add debug option to SystemSettings. and error page shows stacktrace only for admin or debug settings. --- .../controller/SystemSettingsController.scala | 5 +++-- .../core/service/SystemSettingsService.scala | 8 +++++-- .../twirl/gitbucket/core/error.scala.html | 22 +++++++++++-------- .../core/view/AvatarImageProviderSpec.scala | 3 ++- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala index 784172180..2e54423f6 100644 --- a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala @@ -62,8 +62,9 @@ trait SystemSettingsControllerBase extends AccountManagementControllerBase { "mailAttribute" -> trim(label("Mail address attribute", optional(text()))), "tls" -> trim(label("Enable TLS", optional(boolean()))), "ssl" -> trim(label("Enable SSL", optional(boolean()))), - "keystore" -> trim(label("Keystore", optional(text()))) - )(Ldap.apply)) + "keystore" -> trim(label("Keystore", optional(text()))), + )(Ldap.apply)), + "debug" -> trim(label("Debug", boolean())) )(SystemSettings.apply).verifying { settings => Vector( if(settings.ssh && settings.baseUrl.isEmpty){ diff --git a/src/main/scala/gitbucket/core/service/SystemSettingsService.scala b/src/main/scala/gitbucket/core/service/SystemSettingsService.scala index 248d12244..7734f3e8a 100644 --- a/src/main/scala/gitbucket/core/service/SystemSettingsService.scala +++ b/src/main/scala/gitbucket/core/service/SystemSettingsService.scala @@ -54,6 +54,7 @@ trait SystemSettingsService { ldap.keystore.foreach(x => props.setProperty(LdapKeystore, x)) } } + props.setProperty(Debug, settings.debug.toString) using(new java.io.FileOutputStream(GitBucketConf)){ out => props.store(out, null) } @@ -111,7 +112,8 @@ trait SystemSettingsService { getOptionValue(props, LdapKeystore, None))) } else { None - } + }, + getValue(props, Debug, false) ) } } @@ -136,7 +138,8 @@ object SystemSettingsService { useSMTP: Boolean, smtp: Option[Smtp], ldapAuthentication: Boolean, - ldap: Option[Ldap]){ + ldap: Option[Ldap], + debug: Boolean){ def baseUrl(request: HttpServletRequest): String = baseUrl.fold(request.baseUrl)(_.stripSuffix("/")) def sshAddress:Option[SshAddress] = @@ -219,6 +222,7 @@ object SystemSettingsService { private val LdapTls = "ldap.tls" private val LdapSsl = "ldap.ssl" private val LdapKeystore = "ldap.keystore" + private val Debug = "debug" private def getValue[A: ClassTag](props: java.util.Properties, key: String, default: A): A = { getSystemProperty(key).getOrElse(getEnvironmentVariable(key).getOrElse { diff --git a/src/main/twirl/gitbucket/core/error.scala.html b/src/main/twirl/gitbucket/core/error.scala.html index 211073cbc..267179705 100644 --- a/src/main/twirl/gitbucket/core/error.scala.html +++ b/src/main/twirl/gitbucket/core/error.scala.html @@ -3,15 +3,19 @@

@title

- @e.map { ex => -

@ex.getMessage

- - - @ex.getStackTrace.map{ st => - - } - -
@st
+ @if(context.loginAccount.map{_.isAdmin}.getOrElse(false) || context.settings.debug){ + @e.map { ex => +

@ex.getMessage

+ + + @ex.getStackTrace.map{ st => + + } + +
@st
+ } + } else { +
Please contact your administrator.
}
diff --git a/src/test/scala/gitbucket/core/view/AvatarImageProviderSpec.scala b/src/test/scala/gitbucket/core/view/AvatarImageProviderSpec.scala index 27d8c7de5..c78b205ed 100644 --- a/src/test/scala/gitbucket/core/view/AvatarImageProviderSpec.scala +++ b/src/test/scala/gitbucket/core/view/AvatarImageProviderSpec.scala @@ -118,7 +118,8 @@ class AvatarImageProviderSpec extends FunSpec with MockitoSugar { useSMTP = false, smtp = None, ldapAuthentication = false, - ldap = None) + ldap = None, + debug = false) /** * Adapter to test AvatarImageProviderImpl.