diff --git a/internal/federation/authenticate.go b/internal/federation/authenticate.go index 3144d9d05..00fcdfaa9 100644 --- a/internal/federation/authenticate.go +++ b/internal/federation/authenticate.go @@ -216,7 +216,7 @@ func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedU } log.Tracef("proceeding with dereference for uncached public key %s", requestingPublicKeyID) - trans, err := f.transportController.NewTransportForUsername(ctx, requestedUsername) + trans, err := f.transportController.NewTransportForUsername(transport.WithFastfail(ctx), requestedUsername) if err != nil { errWithCode := gtserror.NewErrorInternalError(fmt.Errorf("error creating transport for %s: %s", requestedUsername, err)) log.Debug(errWithCode) diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index 6e107a11d..e351cd2fd 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -87,6 +87,11 @@ type GetRemoteAccountParams struct { // // If a local account is passed into this function for whatever reason (hey, it happens!), then it // will be returned from the database without making any remote calls. +// +// Even if a fastfail context is used, and something goes wrong, an account might still be returned instead +// of an error, if we already had the account in our database (in other words, if we just needed to try +// fingering/refreshing the account again). The rationale for this is that it's more useful to be able +// to provide *something* to the caller, even if that something is not necessarily 100% up to date. func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountParams) (foundAccount *gtsmodel.Account, err error) { /* In this function we want to retrieve a gtsmodel representation of a remote account, with its proper diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index ef64f4050..dbec47f3d 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -34,6 +34,7 @@ "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/log" + "github.com/superseriousbusiness/gotosocial/internal/transport" "github.com/superseriousbusiness/gotosocial/internal/uris" "github.com/superseriousbusiness/gotosocial/internal/util" ) @@ -191,7 +192,7 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr } // we don't have an entry for this instance yet so dereference it - i, err = f.GetRemoteInstance(ctx, username, &url.URL{ + i, err = f.GetRemoteInstance(transport.WithFastfail(ctx), username, &url.URL{ Scheme: publicKeyOwnerURI.Scheme, Host: publicKeyOwnerURI.Host, }) @@ -205,7 +206,7 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr } } - requestingAccount, err := f.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := f.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: username, RemoteAccountID: publicKeyOwnerURI, }) diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 8792e5b82..95071534c 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -95,7 +95,7 @@ func New(cfg Config) *Client { var c Client d := &net.Dialer{ - Timeout: 30 * time.Second, + Timeout: 15 * time.Second, KeepAlive: 30 * time.Second, Resolver: &net.Resolver{}, } diff --git a/internal/processing/account/get.go b/internal/processing/account/get.go index 7d373bc8c..2971dcd2b 100644 --- a/internal/processing/account/get.go +++ b/internal/processing/account/get.go @@ -29,6 +29,7 @@ "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/transport" ) func (p *processor) Get(ctx context.Context, requestingAccount *gtsmodel.Account, targetAccountID string) (*apimodel.Account, gtserror.WithCode) { @@ -93,7 +94,7 @@ func (p *processor) getAccountFor(ctx context.Context, requestingAccount *gtsmod return nil, gtserror.NewErrorInternalError(fmt.Errorf("error parsing url %s: %s", targetAccount.URI, err)) } - a, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + a, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestingAccount.Username, RemoteAccountID: targetAccountURI, RemoteAccountHost: targetAccount.Domain, diff --git a/internal/processing/federation/getfollowers.go b/internal/processing/federation/getfollowers.go index 89c229706..22cffdc73 100644 --- a/internal/processing/federation/getfollowers.go +++ b/internal/processing/federation/getfollowers.go @@ -26,6 +26,7 @@ "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/transport" ) func (p *processor) GetFollowers(ctx context.Context, requestedUsername string, requestURL *url.URL) (interface{}, gtserror.WithCode) { @@ -41,7 +42,7 @@ func (p *processor) GetFollowers(ctx context.Context, requestedUsername string, return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getfollowing.go b/internal/processing/federation/getfollowing.go index 91f097817..c1c7d208e 100644 --- a/internal/processing/federation/getfollowing.go +++ b/internal/processing/federation/getfollowing.go @@ -26,6 +26,7 @@ "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/transport" ) func (p *processor) GetFollowing(ctx context.Context, requestedUsername string, requestURL *url.URL) (interface{}, gtserror.WithCode) { @@ -41,7 +42,7 @@ func (p *processor) GetFollowing(ctx context.Context, requestedUsername string, return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getoutbox.go b/internal/processing/federation/getoutbox.go index 1a9afc14f..67e7f65d7 100644 --- a/internal/processing/federation/getoutbox.go +++ b/internal/processing/federation/getoutbox.go @@ -27,6 +27,7 @@ "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/transport" ) func (p *processor) GetOutbox(ctx context.Context, requestedUsername string, page bool, maxID string, minID string, requestURL *url.URL) (interface{}, gtserror.WithCode) { @@ -42,7 +43,7 @@ func (p *processor) GetOutbox(ctx context.Context, requestedUsername string, pag return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getstatus.go b/internal/processing/federation/getstatus.go index eff8881d8..bbd86b74c 100644 --- a/internal/processing/federation/getstatus.go +++ b/internal/processing/federation/getstatus.go @@ -26,6 +26,7 @@ "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/transport" ) func (p *processor) GetStatus(ctx context.Context, requestedUsername string, requestedStatusID string, requestURL *url.URL) (interface{}, gtserror.WithCode) { @@ -41,7 +42,7 @@ func (p *processor) GetStatus(ctx context.Context, requestedUsername string, req return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getstatusreplies.go b/internal/processing/federation/getstatusreplies.go index 067fcb36b..9575a7b36 100644 --- a/internal/processing/federation/getstatusreplies.go +++ b/internal/processing/federation/getstatusreplies.go @@ -28,6 +28,7 @@ "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/transport" ) func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername string, requestedStatusID string, page bool, onlyOtherAccounts bool, minID string, requestURL *url.URL) (interface{}, gtserror.WithCode) { @@ -43,7 +44,7 @@ func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername stri return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getuser.go b/internal/processing/federation/getuser.go index c1ad9853b..f203d40c8 100644 --- a/internal/processing/federation/getuser.go +++ b/internal/processing/federation/getuser.go @@ -27,6 +27,7 @@ "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/transport" "github.com/superseriousbusiness/gotosocial/internal/uris" ) @@ -53,7 +54,7 @@ func (p *processor) GetUser(ctx context.Context, requestedUsername string, reque // if we're not already handshaking/dereferencing a remote account, dereference it now if !p.federator.Handshaking(ctx, requestedUsername, requestingAccountURI) { - requestingAccount, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/media/getfile.go b/internal/processing/media/getfile.go index 522d47435..ddc14479a 100644 --- a/internal/processing/media/getfile.go +++ b/internal/processing/media/getfile.go @@ -31,6 +31,7 @@ "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/media" + "github.com/superseriousbusiness/gotosocial/internal/transport" "github.com/superseriousbusiness/gotosocial/internal/uris" ) @@ -141,11 +142,11 @@ func (p *processor) getAttachmentContent(ctx context.Context, requestingAccount // large version and derive a thumbnail from it, so use the normal recaching procedure: fetch the media, // process it, then return the thumbnail data data = func(innerCtx context.Context) (io.ReadCloser, int64, error) { - transport, err := p.transportController.NewTransportForUsername(innerCtx, requestingUsername) + t, err := p.transportController.NewTransportForUsername(innerCtx, requestingUsername) if err != nil { return nil, 0, err } - return transport.DereferenceMedia(innerCtx, remoteMediaIRI) + return t.DereferenceMedia(transport.WithFastfail(innerCtx), remoteMediaIRI) } } else { // if it's the full-sized version being requested, we can cheat a bit by streaming data to the user as @@ -172,12 +173,12 @@ func (p *processor) getAttachmentContent(ctx context.Context, requestingAccount attachmentContent.Content = io.NopCloser(bufferedReader) data = func(innerCtx context.Context) (io.ReadCloser, int64, error) { - transport, err := p.transportController.NewTransportForUsername(innerCtx, requestingUsername) + t, err := p.transportController.NewTransportForUsername(innerCtx, requestingUsername) if err != nil { return nil, 0, err } - readCloser, fileSize, err := transport.DereferenceMedia(innerCtx, remoteMediaIRI) + readCloser, fileSize, err := t.DereferenceMedia(transport.WithFastfail(innerCtx), remoteMediaIRI) if err != nil { return nil, 0, err } diff --git a/internal/processing/search.go b/internal/processing/search.go index 8bb2224a5..bc2bc93d4 100644 --- a/internal/processing/search.go +++ b/internal/processing/search.go @@ -34,6 +34,7 @@ "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/internal/transport" "github.com/superseriousbusiness/gotosocial/internal/util" ) @@ -161,13 +162,13 @@ func (p *processor) searchStatusByURI(ctx context.Context, authed *oauth.Auth, u if resolve { // This is a non-local status and we're allowed to resolve, so dereference it - status, statusable, err := p.federator.GetRemoteStatus(ctx, authed.Account.Username, uri, true, true) + status, statusable, err := p.federator.GetRemoteStatus(transport.WithFastfail(ctx), authed.Account.Username, uri, true, true) if err != nil { return nil, fmt.Errorf("searchStatusByURI: error fetching remote status %q: %v", uriStr, err) } // Attempt to dereference the status thread while we are here - p.federator.DereferenceRemoteThread(ctx, authed.Account.Username, uri, status, statusable) + p.federator.DereferenceRemoteThread(transport.WithFastfail(ctx), authed.Account.Username, uri, status, statusable) } return nil, nil @@ -190,7 +191,7 @@ func (p *processor) searchAccountByURI(ctx context.Context, authed *oauth.Auth, } // we don't have it yet, try to find it remotely - return p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + return p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: authed.Account.Username, RemoteAccountID: uri, Blocking: true, @@ -209,7 +210,7 @@ func (p *processor) searchAccountByMention(ctx context.Context, authed *oauth.Au } // we don't have it yet, try to find it remotely - return p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + return p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: authed.Account.Username, RemoteAccountUsername: username, RemoteAccountHost: domain, diff --git a/internal/processing/util.go b/internal/processing/util.go index e360428cc..8b1399d3c 100644 --- a/internal/processing/util.go +++ b/internal/processing/util.go @@ -28,6 +28,7 @@ "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/id" + "github.com/superseriousbusiness/gotosocial/internal/transport" "github.com/superseriousbusiness/gotosocial/internal/util" ) @@ -56,7 +57,7 @@ func GetParseMentionFunc(dbConn db.DB, federator federation.Federator) gtsmodel. if originAccount.Domain == "" { requestingUsername = originAccount.Username } - remoteAccount, err := federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + remoteAccount, err := federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ RequestingUsername: requestingUsername, RemoteAccountUsername: username, RemoteAccountHost: domain, diff --git a/internal/transport/context.go b/internal/transport/context.go new file mode 100644 index 000000000..3f22d4ec8 --- /dev/null +++ b/internal/transport/context.go @@ -0,0 +1,43 @@ +/* + GoToSocial + Copyright (C) 2021-2022 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 . +*/ + +package transport + +import "context" + +// ctxkey is our own unique context key type to prevent setting outside package. +type ctxkey string + +// fastfailkey is our unique context key to indicate fast-fail is enabled. +var fastfailkey = ctxkey("ff") + +// WithFastfail returns a Context which indicates that any http requests made +// with it should return after the first failed attempt, instead of retrying. +// +// This can be used to fail quickly when you're making an outgoing http request +// inside the context of an incoming http request, and you want to be able to +// provide a snappy response to the user, instead of retrying + backing off. +func WithFastfail(parent context.Context) context.Context { + return context.WithValue(parent, fastfailkey, struct{}{}) +} + +// IsFastfail returns true if the given context was created by WithFastfail. +func IsFastfail(ctx context.Context) bool { + _, ok := ctx.Value(fastfailkey).(struct{}) + return ok +} diff --git a/internal/transport/context_test.go b/internal/transport/context_test.go new file mode 100644 index 000000000..c2b099173 --- /dev/null +++ b/internal/transport/context_test.go @@ -0,0 +1,34 @@ +/* + GoToSocial + Copyright (C) 2021-2022 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 . +*/ + +package transport_test + +import ( + "context" + "testing" + + "github.com/superseriousbusiness/gotosocial/internal/transport" +) + +func TestFastFailContext(t *testing.T) { + ctx := context.Background() + ctx = transport.WithFastfail(ctx) + if !transport.IsFastfail(ctx) { + t.Fatal("failed to set fast-fail context key") + } +} diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 94a0cc8ef..d62701ecc 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -23,6 +23,7 @@ "crypto" "crypto/x509" "errors" + "fmt" "io" "net/http" "net/url" @@ -33,25 +34,41 @@ errorsv2 "codeberg.org/gruf/go-errors/v2" "codeberg.org/gruf/go-kv" "github.com/go-fed/httpsig" - "github.com/superseriousbusiness/activity/pub" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/httpclient" "github.com/superseriousbusiness/gotosocial/internal/log" ) -// Transport wraps the pub.Transport interface with some additional functionality for fetching remote media. +// Transport implements the pub.Transport interface with some additional functionality for fetching remote media. // // Since the transport has the concept of 'shortcuts' for fetching data locally rather than remotely, it is // not *always* the case that calling a Transport function does an http call, but it usually will for remote // hosts or resources for which a shortcut isn't provided by the transport controller (also in this package). +// +// For any of the transport functions, if a Fastfail context is passed in as the first parameter, the function +// will return after the first transport failure, instead of retrying + backing off. type Transport interface { - pub.Transport + /* + POST functions + */ + + // 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 + + /* + GET functions + */ + + // 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, targetDomains string) ([]byte, error) + Finger(ctx context.Context, targetUsername string, targetDomain string) ([]byte, error) } // transport implements the Transport interface @@ -106,6 +123,10 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error, retryO return nil, errors.New("too many failed attempts") } + // Check whether request should fast fail, we check this + // before loop as each context.Value() requires mutex lock. + fastFail := IsFastfail(r.Context()) + // Start a log entry for this request l := log.WithFields(kv.Fields{ {"pubKeyID", t.pubKeyID}, @@ -156,6 +177,9 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error, retryO } else if errors.As(err, &x509.UnknownAuthorityError{}) { // Unknown authority errors we do NOT recover from return nil, err + } else if fastFail { + // on fast-fail, don't bother backoff/retry + return nil, fmt.Errorf("%w (fast fail)", err) } l.Errorf("backing off for %s after http request error: %v", backoff.String(), err)