From 68289420284a52f00990573a17ba274d5431a6d5 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 30 Jan 2019 15:37:40 +0000 Subject: [PATCH] Provide better panic handling This PR gitea'ises the macaron.Recovery() handler meaning that in the event of panic we get proper gitea 500 pages and the stacktrace is logged with the gitea logger. Signed-off-by: Andrew Thornton --- modules/context/context.go | 2 +- modules/context/panic.go | 112 +++++++++++++++++++++++++++++++++++++ routers/routes/routes.go | 3 + templates/status/500.tmpl | 3 +- 4 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 modules/context/panic.go diff --git a/modules/context/context.go b/modules/context/context.go index b9736e56ef303..fb17eb27344ce 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -136,7 +136,7 @@ func (ctx *Context) ServerError(title string, err error) { } ctx.Data["Title"] = "Internal Server Error" - ctx.HTML(404, base.TplName("status/500")) + ctx.HTML(http.StatusInternalServerError, base.TplName("status/500")) } // NotFoundOrServerError use error check function to determine if the error diff --git a/modules/context/panic.go b/modules/context/panic.go new file mode 100644 index 0000000000000..810c31a0655a0 --- /dev/null +++ b/modules/context/panic.go @@ -0,0 +1,112 @@ +// Copyright 2013 Martini Authors +// Copyright 2014 The Macaron Authors +// Copyright 2019 The Gitea Authors. All rights reserved. +// +// Licensed 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 context + +import ( + "bytes" + "fmt" + "io/ioutil" + "runtime" + + macaron "gopkg.in/macaron.v1" +) + +// Recovery returns a middleware that recovers from any panics and writes a 500 and a log if so. +// Although similar to macaron.Recovery() the main difference is that this error will be created +// with the gitea 500 page. +func Recovery() macaron.Handler { + return func(ctx *Context) { + defer func() { + if err := recover(); err != nil { + combinedErr := fmt.Errorf("%s\n%s", err, string(stack(3))) + ctx.ServerError("PANIC:", combinedErr) + } + }() + + ctx.Next() + } +} + +var ( + unknown = []byte("???") +) + +// Although we could just use debug.Stack(), this routine will return the source code +// skip the provided number of frames - i.e. allowing us to ignore this function call +// and the preceding function call. +// If the problem is a lack of memory of course all this is not going to work... +func stack(skip int) []byte { + buf := new(bytes.Buffer) + + // Store the last file we opened as its probable that the preceding stack frame + // will be in the same file + var lines [][]byte + var lastFilename string + for i := skip; ; i++ { // Skip over frames + programCounter, filename, lineNumber, ok := runtime.Caller(i) + // If we can't retrieve the information break - basically we're into go internals at this point. + if !ok { + break + } + + // Print equivalent of debug.Stack() + fmt.Fprintf(buf, "%s:%d (0x%x)\n", filename, lineNumber, programCounter) + // Now try to print the offending line + if filename != lastFilename { + data, err := ioutil.ReadFile(filename) + if err != nil { + // can't read this sourcefile + // likely we don't have the sourcecode available + continue + } + lines = bytes.Split(data, []byte{'\n'}) + lastFilename = filename + } + fmt.Fprintf(buf, "\t%s: %s\n", functionName(programCounter), source(lines, lineNumber)) + } + return buf.Bytes() +} + +// functionName converts the provided programCounter into a function name +func functionName(programCounter uintptr) []byte { + function := runtime.FuncForPC(programCounter) + if function == nil { + return unknown + } + name := []byte(function.Name()) + + // Because we provide the filename we can drop the preceding package name. + if lastslash := bytes.LastIndex(name, []byte("/")); lastslash >= 0 { + name = name[lastslash+1:] + } + // And the current package name. + if period := bytes.Index(name, []byte(".")); period >= 0 { + name = name[period+1:] + } + // And we should just replace the interpunct with a dot + name = bytes.Replace(name, []byte("ยท"), []byte("."), -1) + return name +} + +// source returns a space-trimmed slice of the n'th line. +func source(lines [][]byte, n int) []byte { + n-- // in stack trace, lines are 1-indexed but our array is 0-indexed + if n < 0 || n >= len(lines) { + return unknown + } + return bytes.TrimSpace(lines[n]) +} diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 6445c0a9af5f7..681ff38bf0b24 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -136,6 +136,9 @@ func NewMacaron() *macaron.Macaron { DisableDebug: !setting.EnablePprof, })) m.Use(context.Contexter()) + // OK we are now set-up enough to allow us to create a nicer recovery than + // the default macaron recovery + m.Use(context.Recovery()) m.SetAutoHead(true) return m } diff --git a/templates/status/500.tmpl b/templates/status/500.tmpl index 6dfc1d7ace639..2e01e8ac742c4 100644 --- a/templates/status/500.tmpl +++ b/templates/status/500.tmpl @@ -3,7 +3,8 @@

500


- {{if .ErrorMsg}}

An error has occurred : {{.ErrorMsg}}

{{end}} + {{if .ErrorMsg}}

An error has occurred :

+ 
{{.ErrorMsg}}
{{end}} {{if .ShowFooterVersion}}

Application Version: {{AppVer}}

{{end}} {{if .IsAdmin}}

If you are sure this is Gitea bug, please search for issue on GitHub and open new issue if necessary.

{{end}}