gtsocial-umbx

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | README | LICENSE

commit d8d5818b47009f28433a7e96bcce8d116c8a9769
parent f518f649f800e52d32fe53580c528ffa042f7154
Author: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com>
Date:   Mon,  6 Mar 2023 09:38:43 +0000

[bugfix] internal server error on search not found (#1590)

* add error value wrapping, include status code / not found flags from transport errors, update error usages

Signed-off-by: kim <grufwub@gmail.com>

* add code commenting for gtserror functions

Signed-off-by: kim <grufwub@gmail.com>

---------

Signed-off-by: kim <grufwub@gmail.com>
Diffstat:
Minternal/federation/authenticate.go | 3++-
Minternal/federation/dereferencing/error.go | 27+++++++++++++--------------
Minternal/federation/federatingprotocol.go | 3++-
Ainternal/gtserror/error.go | 60++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Minternal/transport/deliver.go | 4+++-
Minternal/transport/dereference.go | 20++++++--------------
Minternal/transport/derefinstance.go | 10+++++++---
Minternal/transport/derefmedia.go | 5++++-
Minternal/transport/transport.go | 13+++++++++++--
9 files changed, 108 insertions(+), 37 deletions(-)

diff --git a/internal/federation/authenticate.go b/internal/federation/authenticate.go @@ -25,6 +25,7 @@ import ( "encoding/pem" "errors" "fmt" + "net/http" "net/url" "strings" @@ -226,7 +227,7 @@ func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedU // The actual http call to the remote server is made right here in the Dereference function. b, err := trans.Dereference(ctx, requestingPublicKeyID) if err != nil { - if errors.Is(err, transport.ErrGone) { + if gtserror.StatusCode(err) == http.StatusGone { // if we get a 410 error it means the account that owns this public key has been deleted; // we should add a tombstone to our database so that we can avoid trying to deref it in future if err := f.HandleGone(ctx, requestingPublicKeyID); err != nil { diff --git a/internal/federation/dereferencing/error.go b/internal/federation/dereferencing/error.go @@ -19,10 +19,10 @@ package dereferencing import ( - "errors" "fmt" + "net/http" - "github.com/superseriousbusiness/gotosocial/internal/transport" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" ) // ErrDB denotes that a proper error has occurred when doing @@ -96,22 +96,21 @@ func newErrOther(err error) error { } func wrapDerefError(derefErr error, fluff string) error { - var ( - err error - errWrongType *ErrWrongType - ) - + // Wrap with fluff. + err := derefErr if fluff != "" { err = fmt.Errorf("%s: %w", fluff, derefErr) } - switch { - case errors.Is(derefErr, transport.ErrGone): - err = NewErrNotRetrievable(err) - case errors.As(derefErr, &errWrongType): - err = newErrWrongType(err) - default: - err = newErrTransportError(err) + // Check for unretrievable HTTP status code errors. + if code := gtserror.StatusCode(derefErr); // nocollapse + code == http.StatusGone || code == http.StatusNotFound { + return NewErrNotRetrievable(err) + } + + // Check for other untrievable errors. + if gtserror.NotFound(derefErr) { + return NewErrNotRetrievable(err) } return err diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go @@ -31,6 +31,7 @@ import ( "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/transport" @@ -210,7 +211,7 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr transport.WithFastfail(ctx), username, publicKeyOwnerURI, false, ) if err != nil { - if errors.Is(err, transport.ErrGone) { + if gtserror.StatusCode(err) == http.StatusGone { // This is the same case as the http.StatusGone check above. // It can happen here and not there because there's a race // where the sending server starts sending account deletion diff --git a/internal/gtserror/error.go b/internal/gtserror/error.go @@ -0,0 +1,60 @@ +/* + GoToSocial + Copyright (C) 2021-2023 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +package gtserror + +import ( + "codeberg.org/gruf/go-errors/v2" +) + +// package private error key type. +type errkey int + +const ( + // error value keys. + _ errkey = iota + statusCodeKey + notFoundKey +) + +// StatusCode checks error for a stored status code value. For example +// an error from an outgoing HTTP request may be stored, or an API handler +// expected response status code may be stored. +func StatusCode(err error) int { + i, _ := errors.Value(err, statusCodeKey).(int) + return i +} + +// WithStatusCode will wrap the given error to store provided status code, +// returning wrapped error. See StatusCode() for example use-cases. +func WithStatusCode(err error, code int) error { + return errors.WithValue(err, statusCodeKey, code) +} + +// NotFound checks error for a stored "not found" flag. For example +// an error from an outgoing HTTP request due to DNS lookup. +func NotFound(err error) bool { + _, ok := errors.Value(err, notFoundKey).(struct{}) + return ok +} + +// SetNotFound will wrap the given error to store a "not found" flag, +// returning wrapped error. See NotFound() for example use-cases. +func SetNotFound(err error) error { + return errors.WithValue(err, notFoundKey, struct{}{}) +} diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go @@ -29,6 +29,7 @@ import ( "codeberg.org/gruf/go-byteutil" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" ) func (t *transport) BatchDeliver(ctx context.Context, b []byte, recipients []*url.URL) error { @@ -96,7 +97,8 @@ func (t *transport) Deliver(ctx context.Context, b []byte, to *url.URL) error { if code := resp.StatusCode; code != http.StatusOK && code != http.StatusCreated && code != http.StatusAccepted { - return fmt.Errorf("POST request to %s failed: %s", urlStr, resp.Status) + err := fmt.Errorf("POST request to %s failed: %s", urlStr, resp.Status) + return gtserror.WithStatusCode(err, resp.StatusCode) } return nil diff --git a/internal/transport/dereference.go b/internal/transport/dereference.go @@ -20,7 +20,6 @@ package transport import ( "context" - "errors" "fmt" "io" "net/http" @@ -28,15 +27,10 @@ import ( apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/uris" ) -// ErrGone is returned from Dereference when the remote resource returns 410 GONE. -// This is useful in cases where we're processing a delete of a resource that's already -// been removed from the remote server, so we know we don't need to keep trying to -// dereference it. -var ErrGone = errors.New("remote resource returned HTTP code 410 GONE") - func (t *transport) Dereference(ctx context.Context, iri *url.URL) ([]byte, error) { // if the request is to us, we can shortcut for certain URIs rather than going through // the normal request flow, thereby saving time and energy @@ -72,12 +66,10 @@ func (t *transport) Dereference(ctx context.Context, iri *url.URL) ([]byte, erro } defer rsp.Body.Close() - switch rsp.StatusCode { - case http.StatusOK: - return io.ReadAll(rsp.Body) - case http.StatusGone: - return nil, ErrGone - default: - return nil, fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) + if rsp.StatusCode != http.StatusOK { + err := fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) + return nil, gtserror.WithStatusCode(err, rsp.StatusCode) } + + return io.ReadAll(rsp.Body) } diff --git a/internal/transport/derefinstance.go b/internal/transport/derefinstance.go @@ -30,6 +30,7 @@ import ( apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/log" @@ -102,7 +103,8 @@ func dereferenceByAPIV1Instance(ctx context.Context, t *transport, iri *url.URL) defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) + err := fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) + return nil, gtserror.WithStatusCode(err, resp.StatusCode) } b, err := io.ReadAll(resp.Body) @@ -252,7 +254,8 @@ func callNodeInfoWellKnown(ctx context.Context, t *transport, iri *url.URL) (*ur defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("callNodeInfoWellKnown: GET request to %s failed: %s", iriStr, resp.Status) + err := fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) + return nil, gtserror.WithStatusCode(err, resp.StatusCode) } b, err := io.ReadAll(resp.Body) @@ -303,7 +306,8 @@ func callNodeInfo(ctx context.Context, t *transport, iri *url.URL) (*apimodel.No defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("callNodeInfo: GET request to %s failed: %s", iriStr, resp.Status) + err := fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) + return nil, gtserror.WithStatusCode(err, resp.StatusCode) } b, err := io.ReadAll(resp.Body) diff --git a/internal/transport/derefmedia.go b/internal/transport/derefmedia.go @@ -24,6 +24,8 @@ import ( "io" "net/http" "net/url" + + "github.com/superseriousbusiness/gotosocial/internal/gtserror" ) func (t *transport) DereferenceMedia(ctx context.Context, iri *url.URL) (io.ReadCloser, int64, error) { @@ -46,7 +48,8 @@ func (t *transport) DereferenceMedia(ctx context.Context, iri *url.URL) (io.Read // Check for an expected status code if rsp.StatusCode != http.StatusOK { - return nil, 0, fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) + err := fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) + return nil, 0, gtserror.WithStatusCode(err, rsp.StatusCode) } return rsp.Body, rsp.ContentLength, nil diff --git a/internal/transport/transport.go b/internal/transport/transport.go @@ -25,6 +25,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" "strconv" @@ -36,6 +37,7 @@ import ( errorsv2 "codeberg.org/gruf/go-errors/v2" "codeberg.org/gruf/go-kv" "github.com/go-fed/httpsig" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/log" @@ -56,6 +58,7 @@ type Transport interface { // Deliver sends an ActivityStreams object. Deliver(ctx context.Context, b []byte, to *url.URL) error + // BatchDeliver sends an ActivityStreams object to multiple recipients. BatchDeliver(ctx context.Context, b []byte, recipients []*url.URL) error @@ -65,10 +68,13 @@ type Transport interface { // Dereference fetches the ActivityStreams object located at this IRI with a GET request. Dereference(ctx context.Context, iri *url.URL) ([]byte, error) + // DereferenceMedia fetches the given media attachment IRI, returning the reader and filesize. DereferenceMedia(ctx context.Context, iri *url.URL) (io.ReadCloser, int64, error) + // DereferenceInstance dereferences remote instance information, first by checking /api/v1/instance, and then by checking /.well-known/nodeinfo. DereferenceInstance(ctx context.Context, iri *url.URL) (*gtsmodel.Instance, error) + // Finger performs a webfinger request with the given username and domain, and returns the bytes from the response body. Finger(ctx context.Context, targetUsername string, targetDomain string) ([]byte, error) } @@ -207,6 +213,10 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error) (*http } else if errors.As(err, &x509.UnknownAuthorityError{}) { // Unknown authority errors we do NOT recover from return nil, err + } else if dnserr := (*net.DNSError)(nil); // nocollapse + errors.As(err, &dnserr) && dnserr.IsNotFound { + // DNS lookup failure, this domain does not exist + return nil, gtserror.SetNotFound(err) } if fastFail { @@ -219,7 +229,7 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error) (*http backoff = time.Duration(i) * baseBackoff } - l.Errorf("backing off for %s after http request error: %v", backoff.String(), err) + l.Errorf("backing off for %s after http request error: %v", backoff, err) select { // Request ctx cancelled @@ -228,7 +238,6 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error) (*http // Backoff for some time case <-time.After(backoff): - backoff *= 2 } }