From ca00ca8ee450243b322457a0512ff338b6672494 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 30 Jan 2019 22:00:00 +0000 Subject: [PATCH] Provide better panic handling (#5902) 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 421d1d5f4f..0faa5c495c 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 0000000000..810c31a065 --- /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 c6b7a097ea..fad2724d2f 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 6dfc1d7ace..2e01e8ac74 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}}