[bugfix] Add back removed ValidateRequest() before backoff-retry loop (#1805)v0.9.0-rc2

* add back removed ValidateRequest() before backoff-retry loop

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

* include response body in error response log

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

* improved error response body draining

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

* add more code commenting

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

* move new error response logic to gtserror, handle instead in transport.Transport{} impl

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

* appease ye oh mighty linter

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

* fix mockhttpclient not setting request in http response

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

---------

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

5 files changed, 19 insertions, 25 deletions

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