rpc: handle panics during panic handling

Fixes #4802. The Go HTTP server has a global panic handler for requests, so it was not as severe as first thought.

This fix can still panic, since we try to send a `500` response - if that happens, the Go HTTP server will terminate the connection. Otherwise, the client will get a 200 response, which we should avoid. I'm sort of torn on whether it's even necessary to include this fix, instead of just letting the HTTP server deal with it.

CHANGELOG_PENDING.md

@@ -53,4 +53,5 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [blockchain/v2] [\#4761](https://github.com/tendermint/tendermint/pull/4761) Fix excessive CPU usage caused by spinning on closed channels (@erikgrinaker)
 - [blockchain/v2] Respect `fast_sync` option (@erikgrinaker)
 - [light] [\#4741](https://github.com/tendermint/tendermint/pull/4741) Correctly return  `ErrSignedHeaderNotFound` and `ErrValidatorSetNotFound` on corresponding RPC errors (@erikgrinaker)
+- [rpc] \#4805 Attempt to handle panics during panic recovery (@erikgrinaker)
 - [types] [\#4764](https://github.com/tendermint/tendermint/pull/4764) Return an error if voting power overflows in `VerifyCommitTrusting` (@melekes)

rpc/lib/server/http_server.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"os"
 	"runtime/debug"
 	"strings"
 	"time"
@@ -145,6 +146,20 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
 
 		rww.Header().Set("X-Server-Time", fmt.Sprintf("%v", begin.Unix()))
 
+		defer func() {
+			// Handle any panics in the panic handler below. Does not use the logger, since we want
+			// to avoid any further panics. However, we try to return a 500, since it otherwise
+			// defaults to 200 and there is no other way to terminate the connection. If that
+			// should panic for whatever reason then the Go HTTP server will handle it and
+			// terminate the connection - panicing is the de-facto and only way to get the Go HTTP
+			// server to terminate the request and close the connection/stream:
+			// https://github.com/golang/go/issues/17790#issuecomment-258481416
+			if e := recover(); e != nil {
+				fmt.Fprintf(os.Stderr, "Panic during RPC panic recovery: %v\n%v\n", e, string(debug.Stack()))
+				w.WriteHeader(500)
+			}
+		}()
+
 		defer func() {
 			// Send a 500 error if a panic happens during a handler.
 			// Without this, Chrome & Firefox were retrying aborted ajax requests,
@@ -155,7 +170,18 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
 				if res, ok := e.(types.RPCResponse); ok {
 					WriteRPCResponseHTTP(rww, res)
 				} else {
-					// For the rest,
+					// Panics can contain anything, attempt to normalize it as an error.
+					var err error
+					switch e := e.(type) {
+					case error:
+						err = e
+					case string:
+						err = errors.New(e)
+					case fmt.Stringer:
+						err = errors.New(e.String())
+					default:
+					}
+
 					logger.Error(
 						"Panic in RPC HTTP handler", "err", e, "stack",
 						string(debug.Stack()),
@@ -163,7 +189,7 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
 					WriteRPCResponseHTTPError(
 						rww,
 						http.StatusInternalServerError,
-						types.RPCInternalError(types.JSONRPCIntID(-1), e.(error)),
+						types.RPCInternalError(types.JSONRPCIntID(-1), err),
 					)
 				}
 			}