From 2063d01cdb02c7ef26dc6d917e3bca252db5d5a8 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Sun, 21 May 2023 17:59:14 +0100 Subject: [bugfix] Add back removed ValidateRequest() before backoff-retry loop (#1805) * add back removed ValidateRequest() before backoff-retry loop Signed-off-by: kim * include response body in error response log Signed-off-by: kim * improved error response body draining Signed-off-by: kim * add more code commenting Signed-off-by: kim * move new error response logic to gtserror, handle instead in transport.Transport{} impl Signed-off-by: kim * appease ye oh mighty linter Signed-off-by: kim * fix mockhttpclient not setting request in http response Signed-off-by: kim --------- Signed-off-by: kim --- internal/transport/deliver.go | 4 +--- internal/transport/dereference.go | 4 +--- internal/transport/derefinstance.go | 11 ++++------- internal/transport/derefmedia.go | 4 +--- internal/transport/finger.go | 21 ++++++++++++--------- 5 files changed, 19 insertions(+), 25 deletions(-) (limited to 'internal/transport') diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index fff7dbcf4..054baa6a5 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -19,7 +19,6 @@ package transport import ( "context" - "fmt" "net/http" "net/url" "sync" @@ -131,8 +130,7 @@ func (t *transport) deliver(ctx context.Context, b []byte, to *url.URL) error { if code := rsp.StatusCode; code != http.StatusOK && code != http.StatusCreated && code != http.StatusAccepted { - err := fmt.Errorf("POST request to %s failed: %s", url, rsp.Status) - return gtserror.WithStatusCode(err, rsp.StatusCode) + return gtserror.NewResponseError(rsp) } return nil diff --git a/internal/transport/dereference.go b/internal/transport/dereference.go index 71b10a0f1..e231e0954 100644 --- a/internal/transport/dereference.go +++ b/internal/transport/dereference.go @@ -19,7 +19,6 @@ package transport import ( "context" - "fmt" "io" "net/http" "net/url" @@ -66,8 +65,7 @@ func (t *transport) Dereference(ctx context.Context, iri *url.URL) ([]byte, erro defer rsp.Body.Close() 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 nil, gtserror.NewResponseError(rsp) } return io.ReadAll(rsp.Body) diff --git a/internal/transport/derefinstance.go b/internal/transport/derefinstance.go index 466981348..c373a140a 100644 --- a/internal/transport/derefinstance.go +++ b/internal/transport/derefinstance.go @@ -102,8 +102,7 @@ func dereferenceByAPIV1Instance(ctx context.Context, t *transport, iri *url.URL) defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - err := fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) - return nil, gtserror.WithStatusCode(err, resp.StatusCode) + return nil, gtserror.NewResponseError(resp) } b, err := io.ReadAll(resp.Body) @@ -133,7 +132,7 @@ func dereferenceByAPIV1Instance(ctx context.Context, t *transport, iri *url.URL) ID: ulid, Domain: iri.Host, Title: apiResp.Title, - URI: fmt.Sprintf("%s://%s", iri.Scheme, iri.Host), + URI: iri.Scheme + "://" + iri.Host, ShortDescription: apiResp.ShortDescription, Description: apiResp.Description, ContactEmail: apiResp.Email, @@ -253,8 +252,7 @@ func callNodeInfoWellKnown(ctx context.Context, t *transport, iri *url.URL) (*ur defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - err := fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) - return nil, gtserror.WithStatusCode(err, resp.StatusCode) + return nil, gtserror.NewResponseError(resp) } b, err := io.ReadAll(resp.Body) @@ -305,8 +303,7 @@ func callNodeInfo(ctx context.Context, t *transport, iri *url.URL) (*apimodel.No defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - err := fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) - return nil, gtserror.WithStatusCode(err, resp.StatusCode) + return nil, gtserror.NewResponseError(resp) } b, err := io.ReadAll(resp.Body) diff --git a/internal/transport/derefmedia.go b/internal/transport/derefmedia.go index 2d9096493..ad47d99b5 100644 --- a/internal/transport/derefmedia.go +++ b/internal/transport/derefmedia.go @@ -19,7 +19,6 @@ package transport import ( "context" - "fmt" "io" "net/http" "net/url" @@ -47,8 +46,7 @@ func (t *transport) DereferenceMedia(ctx context.Context, iri *url.URL) (io.Read // Check for an expected status code if rsp.StatusCode != http.StatusOK { - err := fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) - return nil, 0, gtserror.WithStatusCode(err, rsp.StatusCode) + return nil, 0, gtserror.NewResponseError(rsp) } return rsp.Body, rsp.ContentLength, nil diff --git a/internal/transport/finger.go b/internal/transport/finger.go index 18b028a64..e6086747b 100644 --- a/internal/transport/finger.go +++ b/internal/transport/finger.go @@ -27,6 +27,7 @@ import ( apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" ) // webfingerURLFor returns the URL to try a webfinger request against, as @@ -105,14 +106,16 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom // From here on out, we're handling different failure scenarios and // deciding whether we should do a host-meta based fallback or not - if (rsp.StatusCode >= 500 && rsp.StatusCode < 600) || cached { - // In case we got a 5xx, bail out irrespective of if the value - // was cached or not. The target may be broken or be signalling - // us to back-off. - // - // If it's any error but the URL was cached, bail out too - return nil, fmt.Errorf("GET request to %s failed: %s", req.URL.String(), rsp.Status) - } + // Response status codes >= 500 are returned as errors by the wrapped HTTP client. + // + // if (rsp.StatusCode >= 500 && rsp.StatusCode < 600) || cached { + // In case we got a 5xx, bail out irrespective of if the value + // was cached or not. The target may be broken or be signalling + // us to back-off. + // + // If it's any error but the URL was cached, bail out too + // return nil, gtserror.NewResponseError(rsp) + // } // So far we've failed to get a successful response from the expected // webfinger endpoint. Lets try and discover the webfinger endpoint @@ -153,7 +156,7 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom } // We've reached the end of the line here, both the original request // and our attempt to resolve it through the fallback have failed - return nil, fmt.Errorf("GET request to %s failed: %s", req.URL.String(), rsp.Status) + return nil, gtserror.NewResponseError(rsp) } // Set the URL in cache here, since host-meta told us this should be the -- cgit v1.3