Skip to content

Commit

Permalink
feat: simplify error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@hacdias
hacdias committed Feb 22, 2023
1 parent 6345364 commit 30f26b3
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 74 deletions.
12 changes: 6 additions & 6 deletions gateway/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,17 +349,17 @@ func TestGatewayGet(t *testing.T) {
{"127.0.0.1:8080", "/", http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", "/" + k.Cid().String(), http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", k.String(), http.StatusOK, "fnord"},
{"127.0.0.1:8080", "/ipns/nxdomain.example.com", http.StatusInternalServerError, "ipfs resolve -r /ipns/nxdomain.example.com: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/%0D%0A%0D%0Ahello", http.StatusInternalServerError, "ipfs resolve -r /ipns/\\r\\n\\r\\nhello: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/nxdomain.example.com", http.StatusInternalServerError, "failed to resolve /ipns/nxdomain.example.com: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/%0D%0A%0D%0Ahello", http.StatusInternalServerError, "failed to resolve /ipns/\\r\\n\\r\\nhello: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/example.com", http.StatusOK, "fnord"},
{"example.com", "/", http.StatusOK, "fnord"},

{"working.example.com", "/", http.StatusOK, "fnord"},
{"double.example.com", "/", http.StatusOK, "fnord"},
{"triple.example.com", "/", http.StatusOK, "fnord"},
{"working.example.com", k.String(), http.StatusNotFound, "ipfs resolve -r /ipns/working.example.com" + k.String() + ": no link named \"ipfs\" under " + k.Cid().String() + "\n"},
{"broken.example.com", "/", http.StatusInternalServerError, "ipfs resolve -r /ipns/broken.example.com/: " + namesys.ErrResolveFailed.Error() + "\n"},
{"broken.example.com", k.String(), http.StatusInternalServerError, "ipfs resolve -r /ipns/broken.example.com" + k.String() + ": " + namesys.ErrResolveFailed.Error() + "\n"},
{"working.example.com", k.String(), http.StatusNotFound, "failed to resolve /ipns/working.example.com" + k.String() + ": no link named \"ipfs\" under " + k.Cid().String() + "\n"},
{"broken.example.com", "/", http.StatusInternalServerError, "failed to resolve /ipns/broken.example.com/: " + namesys.ErrResolveFailed.Error() + "\n"},
{"broken.example.com", k.String(), http.StatusInternalServerError, "failed to resolve /ipns/broken.example.com" + k.String() + ": " + namesys.ErrResolveFailed.Error() + "\n"},
// This test case ensures we don't treat the TLD as a file extension.
{"example.man", "/", http.StatusOK, "fnord"},
} {
Expand Down Expand Up @@ -686,7 +686,7 @@ func TestPretty404(t *testing.T) {
{"/nope", "text/html", http.StatusNotFound, "Custom 404"},
{"/nope", "text/*", http.StatusNotFound, "Custom 404"},
{"/nope", "*/*", http.StatusNotFound, "Custom 404"},
{"/nope", "application/json", http.StatusNotFound, fmt.Sprintf("ipfs resolve -r /ipns/example.net/nope: no link named \"nope\" under %s\n", k.Cid().String())},
{"/nope", "application/json", http.StatusNotFound, fmt.Sprintf("failed to resolve /ipns/example.net/nope: no link named \"nope\" under %s\n", k.Cid().String())},
{"/deeper/nope", "text/html", http.StatusNotFound, "Deep custom 404"},
{"/deeper/", "text/html", http.StatusOK, ""},
{"/deeper", "text/html", http.StatusOK, ""},
Expand Down
84 changes: 50 additions & 34 deletions gateway/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ type statusResponseWriter struct {

// Custom type for collecting error details to be handled by `webRequestError`
type requestError struct {
Message string
StatusCode int
Err error
}
Expand All @@ -108,9 +107,8 @@ func (r *requestError) Error() string {
return r.Err.Error()
}

func newRequestError(message string, err error, statusCode int) *requestError {
func newRequestError(err error, statusCode int) *requestError {
return &requestError{
Message: message,
Err: err,
StatusCode: statusCode,
}
Expand Down Expand Up @@ -372,7 +370,7 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) {
// Detect when explicit Accept header or ?format parameter are present
responseFormat, formatParams, err := customResponseFormat(r)
if err != nil {
webError(w, "error while processing the Accept header", err, http.StatusBadRequest)
webError(w, fmt.Errorf("error while processing the Accept header: %w", err), http.StatusBadRequest)
return
}
trace.SpanFromContext(r.Context()).SetAttributes(attribute.String("ResponseFormat", responseFormat))
Expand Down Expand Up @@ -438,7 +436,7 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) {
success = i.serveIpnsRecord(r.Context(), w, r, resolvedPath, contentPath, begin, logger)
default: // catch-all for unsuported application/vnd.*
err := fmt.Errorf("unsupported format %q", responseFormat)
webError(w, "failed to respond with requested content type", err, http.StatusBadRequest)
webError(w, err, http.StatusBadRequest)
return
}

Expand Down Expand Up @@ -558,35 +556,48 @@ func (i *handler) buildIpfsRootsHeader(contentPath string, r *http.Request) (str
}

func webRequestError(w http.ResponseWriter, err *requestError) {
webError(w, err.Message, err.Err, err.StatusCode)
webError(w, err.Err, err.StatusCode)
}

func internalWebError(w http.ResponseWriter, err error) {
webError(w, "internalWebError", err, http.StatusInternalServerError)
}

func webError(w http.ResponseWriter, message string, err error, defaultCode int) {
if _, ok := err.(resolver.ErrNoLink); ok {
webErrorWithCode(w, message, err, http.StatusNotFound)
func webError(w http.ResponseWriter, err error, defaultCode int) {
if isErrNoLink(err) {
webErrorWithCode(w, err, http.StatusNotFound)
} else if errors.Is(err, routing.ErrNotFound) || err == routing.ErrNotFound {
webErrorWithCode(w, message, err, http.StatusNotFound)
webErrorWithCode(w, err, http.StatusNotFound)
} else if ipld.IsNotFound(err) {
webErrorWithCode(w, message, err, http.StatusNotFound)
webErrorWithCode(w, err, http.StatusNotFound)
} else if errors.Is(err, ErrGatewayTimeout) {
webErrorWithCode(w, message, err, http.StatusGatewayTimeout)
webErrorWithCode(w, err, http.StatusGatewayTimeout)
} else if errors.Is(err, ErrBadGateway) {
webErrorWithCode(w, message, err, http.StatusBadGateway)
webErrorWithCode(w, err, http.StatusBadGateway)
} else if errors.Is(err, context.DeadlineExceeded) || err == context.DeadlineExceeded {
webErrorWithCode(w, message, err, http.StatusRequestTimeout)
webErrorWithCode(w, err, http.StatusRequestTimeout)
} else {
webErrorWithCode(w, message, err, defaultCode)
webErrorWithCode(w, err, defaultCode)
}
}

// isErrNoLink checks if err is a resolver.ErrNoLink. resolver.ErrNoLink
// does not implement a .Is interface and cannot be directly compared to.
// Therefore, errors.Is always returns false with it.
func isErrNoLink(err error) bool {
for {
_, ok := err.(resolver.ErrNoLink)
if ok {
return true
}

err = errors.Unwrap(err)
if err == nil {
return false
}
}
}

func webErrorWithCode(w http.ResponseWriter, message string, err error, code int) {
http.Error(w, fmt.Sprintf("%s: %s", message, err), code)
func webErrorWithCode(w http.ResponseWriter, err error, code int) {
http.Error(w, err.Error(), code)
if code >= 500 {
log.Warnf("server error: %s: %s", message, err)
log.Warnf("server error: %s", err)
}
}

Expand Down Expand Up @@ -749,11 +760,13 @@ func (i *handler) handlePathResolution(w http.ResponseWriter, r *http.Request, r
case nil:
return resolvedPath, contentPath, true
case coreiface.ErrOffline:
webError(w, "ipfs resolve -r "+debugStr(contentPath.String()), err, http.StatusServiceUnavailable)
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusServiceUnavailable)
return nil, nil, false
case namesys.ErrResolveFailed:
// Note: webError will replace http.StatusBadRequest with StatusNotFound or StatusRequestTimeout if necessary
webError(w, "ipfs resolve -r "+debugStr(contentPath.String()), err, http.StatusInternalServerError)
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusInternalServerError)
return nil, nil, false
default:
// The path can't be resolved.
Expand All @@ -780,7 +793,8 @@ func (i *handler) handlePathResolution(w http.ResponseWriter, r *http.Request, r
}

// Note: webError will replace http.StatusBadRequest with StatusNotFound or StatusRequestTimeout if necessary
webError(w, "ipfs resolve -r "+debugStr(contentPath.String()), err, http.StatusBadRequest)
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusBadRequest)
return nil, nil, false
}
}
Expand Down Expand Up @@ -810,8 +824,8 @@ func handleUnsupportedHeaders(r *http.Request) (err *requestError) {
// X-Ipfs-Gateway-Prefix was removed (https://github.com/ipfs/kubo/issues/7702)
// TODO: remove this after go-ipfs 0.13 ships
if prfx := r.Header.Get("X-Ipfs-Gateway-Prefix"); prfx != "" {
err := fmt.Errorf("X-Ipfs-Gateway-Prefix support was removed: https://github.com/ipfs/kubo/issues/7702")
return newRequestError("unsupported HTTP header", err, http.StatusBadRequest)
err := fmt.Errorf("unsupported HTTP header: X-Ipfs-Gateway-Prefix support was removed: https://github.com/ipfs/kubo/issues/7702")
return newRequestError(err, http.StatusBadRequest)
}
return nil
}
Expand All @@ -824,11 +838,11 @@ func handleProtocolHandlerRedirect(w http.ResponseWriter, r *http.Request, logge
if uriParam := r.URL.Query().Get("uri"); uriParam != "" {
u, err := url.Parse(uriParam)
if err != nil {
webError(w, "failed to parse uri query parameter", err, http.StatusBadRequest)
webError(w, fmt.Errorf("failed to parse uri query parameter: %w", err), http.StatusBadRequest)
return true
}
if u.Scheme != "ipfs" && u.Scheme != "ipns" {
webError(w, "uri query parameter scheme must be ipfs or ipns", err, http.StatusBadRequest)
webError(w, fmt.Errorf("uri query parameter scheme must be ipfs or ipns: %w", err), http.StatusBadRequest)
return true
}
path := u.Path
Expand All @@ -852,7 +866,7 @@ func handleServiceWorkerRegistration(r *http.Request) (err *requestError) {
matched, _ := regexp.MatchString(`^/ip[fn]s/[^/]+$`, r.URL.Path)
if matched {
err := fmt.Errorf("registration is not allowed for this scope")
return newRequestError("navigator.serviceWorker", err, http.StatusBadRequest)
return newRequestError(fmt.Errorf("navigator.serviceWorker: %w", err), http.StatusBadRequest)
}
}

Expand All @@ -877,7 +891,7 @@ func handleSuperfluousNamespace(w http.ResponseWriter, r *http.Request, contentP
// Attempt to fix the superflous namespace
intendedPath := ipath.New(strings.TrimPrefix(r.URL.Path, "/ipfs"))
if err := intendedPath.IsValid(); err != nil {
webError(w, "invalid ipfs path", err, http.StatusBadRequest)
webError(w, fmt.Errorf("invalid ipfs path: %w", err), http.StatusBadRequest)
return true
}
intendedURL := intendedPath.String()
Expand All @@ -897,7 +911,7 @@ func handleSuperfluousNamespace(w http.ResponseWriter, r *http.Request, contentP
SuggestedPath: intendedPath.String(),
ErrorMsg: fmt.Sprintf("invalid path: %q should be %q", r.URL.Path, intendedPath.String()),
}); err != nil {
webError(w, "failed to redirect when fixing superfluous namespace", err, http.StatusBadRequest)
webError(w, fmt.Errorf("failed to redirect when fixing superfluous namespace: %w", err), http.StatusBadRequest)
}

return true
Expand All @@ -908,7 +922,8 @@ func (i *handler) handleGettingFirstBlock(r *http.Request, begin time.Time, cont
// NOTE: for legacy reasons this happens before we go into content-type specific code paths
_, err := i.api.GetBlock(r.Context(), resolvedPath.Cid())
if err != nil {
return newRequestError("ipfs block get "+resolvedPath.Cid().String(), err, http.StatusInternalServerError)
err = fmt.Errorf("could not get block %s: %w", resolvedPath.Cid().String(), err)
return newRequestError(err, http.StatusInternalServerError)
}
ns := contentPath.Namespace()
timeToGetFirstContentBlock := time.Since(begin).Seconds()
Expand All @@ -924,7 +939,8 @@ func (i *handler) setCommonHeaders(w http.ResponseWriter, r *http.Request, conte
if rootCids, err := i.buildIpfsRootsHeader(contentPath.String(), r); err == nil {
w.Header().Set("X-Ipfs-Roots", rootCids)
} else { // this should never happen, as we resolved the contentPath already
return newRequestError("error while resolving X-Ipfs-Roots", err, http.StatusInternalServerError)
err = fmt.Errorf("error while resolving X-Ipfs-Roots: %s", err)
return newRequestError(err, http.StatusInternalServerError)
}

return nil
Expand Down
4 changes: 3 additions & 1 deletion gateway/handler_block.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package gateway
import (
"bytes"
"context"
"fmt"
"net/http"
"time"

Expand All @@ -19,7 +20,8 @@ func (i *handler) serveRawBlock(ctx context.Context, w http.ResponseWriter, r *h
blockCid := resolvedPath.Cid()
block, err := i.api.GetBlock(ctx, blockCid)
if err != nil {
webError(w, "ipfs block get "+blockCid.String(), err, http.StatusInternalServerError)
err = fmt.Errorf("error getting block %s: %w", blockCid.String(), err)
webError(w, err, http.StatusInternalServerError)
return false
}
content := bytes.NewReader(block.RawData())
Expand Down
4 changes: 2 additions & 2 deletions gateway/handler_car.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R
case "": // noop, client does not care about version
case "1": // noop, we support this
default:
err := fmt.Errorf("only version=1 is supported")
webError(w, "unsupported CAR version", err, http.StatusBadRequest)
err := fmt.Errorf("unsupported CAR version: only version=1 is supported")
webError(w, err, http.StatusBadRequest)
return false
}
rootCid := resolvedPath.Cid()
Expand Down
24 changes: 13 additions & 11 deletions gateway/handler_codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"fmt"
"html"
"net/http"
"strings"
"time"
Expand Down Expand Up @@ -64,7 +63,7 @@ func (i *handler) serveCodec(ctx context.Context, w http.ResponseWriter, r *http
if resolvedPath.Remainder() != "" {
path := strings.TrimSuffix(resolvedPath.String(), resolvedPath.Remainder())
err := fmt.Errorf("%q of %q could not be returned: reading IPLD Kinds other than Links (CBOR Tag 42) is not implemented: try reading %q instead", resolvedPath.Remainder(), resolvedPath.String(), path)
webError(w, "unsupported pathing", err, http.StatusNotImplemented)
webError(w, err, http.StatusNotImplemented)
return false
}

Expand All @@ -74,7 +73,7 @@ func (i *handler) serveCodec(ctx context.Context, w http.ResponseWriter, r *http
if !ok {
// Should not happen unless function is called with wrong parameters.
err := fmt.Errorf("content type not found for codec: %v", cidCodec)
webError(w, "internal error", err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}
responseContentType = cidContentType
Expand Down Expand Up @@ -119,7 +118,7 @@ func (i *handler) serveCodec(ctx context.Context, w http.ResponseWriter, r *http
if !ok {
// This is never supposed to happen unless function is called with wrong parameters.
err := fmt.Errorf("unsupported content type: %s", requestedContentType)
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

Expand Down Expand Up @@ -151,7 +150,8 @@ func (i *handler) serveCodecHTML(ctx context.Context, w http.ResponseWriter, r *
CodecName: cidCodec.String(),
CodecHex: fmt.Sprintf("0x%x", uint64(cidCodec)),
}); err != nil {
webError(w, "failed to generate HTML listing for this DAG: try fetching raw block with ?format=raw", err, http.StatusInternalServerError)
err = fmt.Errorf("failed to generate HTML listing for this DAG: try fetching raw block with ?format=raw: %w", err)
webError(w, err, http.StatusInternalServerError)
return false
}

Expand All @@ -163,7 +163,8 @@ func (i *handler) serveCodecRaw(ctx context.Context, w http.ResponseWriter, r *h
blockCid := resolvedPath.Cid()
block, err := i.api.GetBlock(ctx, blockCid)
if err != nil {
webError(w, "ipfs block get "+blockCid.String(), err, http.StatusInternalServerError)
err = fmt.Errorf("error getting block %s: %w", blockCid.String(), err)
webError(w, err, http.StatusInternalServerError)
return false
}
content := bytes.NewReader(block.RawData())
Expand All @@ -185,35 +186,36 @@ func (i *handler) serveCodecConverted(ctx context.Context, w http.ResponseWriter
blockCid := resolvedPath.Cid()
block, err := i.api.GetBlock(ctx, blockCid)
if err != nil {
webError(w, "ipfs block get "+html.EscapeString(resolvedPath.String()), err, http.StatusInternalServerError)
err = fmt.Errorf("error getting block %s: %w", blockCid.String(), err)
webError(w, err, http.StatusInternalServerError)
return false
}

codec := blockCid.Prefix().Codec
decoder, err := multicodec.LookupDecoder(codec)
if err != nil {
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

node := basicnode.Prototype.Any.NewBuilder()
err = decoder(node, bytes.NewReader(block.RawData()))
if err != nil {
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

encoder, err := multicodec.LookupEncoder(uint64(toCodec))
if err != nil {
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

// Ensure IPLD node conforms to the codec specification.
var buf bytes.Buffer
err = encoder(node.Build(), &buf)
if err != nil {
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

Expand Down
10 changes: 5 additions & 5 deletions gateway/handler_ipns_record.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (i *handler) serveIpnsRecord(ctx context.Context, w http.ResponseWriter, r

if contentPath.Namespace() != "ipns" {
err := fmt.Errorf("%s is not an IPNS link", contentPath.String())
webError(w, err.Error(), err, http.StatusBadRequest)
webError(w, err, http.StatusBadRequest)
return false
}

Expand All @@ -32,26 +32,26 @@ func (i *handler) serveIpnsRecord(ctx context.Context, w http.ResponseWriter, r
key = strings.TrimPrefix(key, "/ipns/")
if strings.Count(key, "/") != 0 {
err := errors.New("cannot find ipns key for subpath")
webError(w, err.Error(), err, http.StatusBadRequest)
webError(w, err, http.StatusBadRequest)
return false
}

c, err := cid.Decode(key)
if err != nil {
webError(w, err.Error(), err, http.StatusBadRequest)
webError(w, err, http.StatusBadRequest)
return false
}

rawRecord, err := i.api.GetIPNSRecord(ctx, c)
if err != nil {
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

var record ipns_pb.IpnsEntry
err = proto.Unmarshal(rawRecord, &record)
if err != nil {
webError(w, err.Error(), err, http.StatusInternalServerError)
webError(w, err, http.StatusInternalServerError)
return false
}

Expand Down
Loading

0 comments on commit 30f26b3

Please sign in to comment.