fix(gateway): 500 on panic, recover on WithHostname

Co-authored-by: Henrique Dias <hacdias@gmail.com>

gateway/handler.go

@@ -273,19 +273,13 @@ func newHandler(c Config, api API) *handler {
 }
 
 func (i *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	defer panicHandler(w)
+
 	// the hour is a hard fallback, we don't expect it to happen, but just in case
 	ctx, cancel := context.WithTimeout(r.Context(), time.Hour)
 	defer cancel()
 	r = r.WithContext(ctx)
 
-	defer func() {
-		if r := recover(); r != nil {
-			log.Error("A panic occurred in the gateway handler!")
-			log.Error(r)
-			debug.PrintStack()
-		}
-	}()
-
 	switch r.Method {
 	case http.MethodGet, http.MethodHead:
 		i.getOrHeadHandler(w, r)
@@ -428,6 +422,15 @@ func (i *handler) addUserHeaders(w http.ResponseWriter) {
 	}
 }
 
+func panicHandler(w http.ResponseWriter) {
+	if r := recover(); r != nil {
+		log.Error("A panic occurred in the gateway handler!")
+		log.Error(r)
+		debug.PrintStack()
+		w.WriteHeader(http.StatusInternalServerError)
+	}
+}
+
 func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath ipath.Path, fileCid cid.Cid) (modtime time.Time) {
 	// Set Etag to based on CID (override whatever was set before)
 	w.Header().Set("Etag", getEtag(r, fileCid))

gateway/handler_test.go

@@ -2,6 +2,7 @@ package gateway
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -140,3 +141,68 @@ func TestErrorBubblingFromAPI(t *testing.T) {
 		assert.Equal(t, test.headerLength, len(res.Header.Values(test.headerName)))
 	}
 }
+
+type panicMockAPI struct {
+	panicOnHostnameHandler bool
+}
+
+func (api *panicMockAPI) GetUnixFsNode(context.Context, ipath.Resolved) (files.Node, error) {
+	panic("i am panicking")
+}
+
+func (api *panicMockAPI) LsUnixFsDir(ctx context.Context, p ipath.Resolved) (<-chan iface.DirEntry, error) {
+	panic("i am panicking")
+}
+
+func (api *panicMockAPI) GetBlock(ctx context.Context, c cid.Cid) (blocks.Block, error) {
+	panic("i am panicking")
+}
+
+func (api *panicMockAPI) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte, error) {
+	panic("i am panicking")
+}
+
+func (api *panicMockAPI) GetDNSLinkRecord(ctx context.Context, hostname string) (ipath.Path, error) {
+	// GetDNSLinkRecord is also called on the WithHostname handler. We have this option
+	// to disable panicking here so we can test if both the regular gateway handler
+	// and the hostname handler can handle panics.
+	if api.panicOnHostnameHandler {
+		panic("i am panicking")
+	}
+
+	return nil, errors.New("not implemented")
+}
+
+func (api *panicMockAPI) IsCached(ctx context.Context, p ipath.Path) bool {
+	panic("i am panicking")
+}
+
+func (api *panicMockAPI) ResolvePath(ctx context.Context, ip ipath.Path) (ipath.Resolved, error) {
+	panic("i am panicking")
+}
+
+func TestGatewayStatusCodeOnPanic(t *testing.T) {
+	api := &panicMockAPI{}
+	ts := newTestServer(t, api)
+	t.Logf("test server url: %s", ts.URL)
+
+	req, err := http.NewRequest(http.MethodGet, ts.URL+"/ipfs/bafkreifzjut3te2nhyekklss27nh3k72ysco7y32koao5eei66wof36n5e", nil)
+	assert.Nil(t, err)
+
+	res, err := ts.Client().Do(req)
+	assert.Nil(t, err)
+	assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
+}
+
+func TestGatewayStatusCodeOnHostnamePanic(t *testing.T) {
+	api := &panicMockAPI{panicOnHostnameHandler: true}
+	ts := newTestServer(t, api)
+	t.Logf("test server url: %s", ts.URL)
+
+	req, err := http.NewRequest(http.MethodGet, ts.URL+"/ipfs/bafkreifzjut3te2nhyekklss27nh3k72ysco7y32koao5eei66wof36n5e", nil)
+	assert.Nil(t, err)
+
+	res, err := ts.Client().Do(req)
+	assert.Nil(t, err)
+	assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
+}

gateway/hostname.go

@@ -61,6 +61,8 @@ func WithHostname(next http.Handler, api API, publicGateways map[string]*Specifi
 	gateways := prepareHostnameGateways(publicGateways)
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer panicHandler(w)
+
 		// Unfortunately, many (well, ipfs.io) gateways use
 		// DNSLink so if we blindly rewrite with DNSLink, we'll
 		// break /ipfs links.