From a684fc46288e38a7e1d3555cb9a5a48b8c65b634 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Sat, 18 Feb 2023 16:02:19 +0000 Subject: [PATCH] [chore] transport improvements (#1524) * improve error readability, mark "bad hosts" as fastFail Signed-off-by: kim * pull in latest go-byteutil version with byteutil.Reader{} Signed-off-by: kim * use rewindable body reader for post requests Signed-off-by: kim --------- Signed-off-by: kim --- go.mod | 2 +- go.sum | 4 +- internal/transport/deliver.go | 12 +++-- internal/transport/dereference.go | 2 +- internal/transport/derefinstance.go | 6 +-- internal/transport/derefmedia.go | 2 +- internal/transport/finger.go | 2 +- internal/transport/transport.go | 30 +++++++----- vendor/codeberg.org/gruf/go-byteutil/bytes.go | 49 +++++-------------- .../codeberg.org/gruf/go-byteutil/reader.go | 36 ++++++++++++++ vendor/modules.txt | 2 +- 11 files changed, 84 insertions(+), 63 deletions(-) create mode 100644 vendor/codeberg.org/gruf/go-byteutil/reader.go diff --git a/go.mod b/go.mod index 9bdbf8fda..e54b38cbe 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( codeberg.org/gruf/go-bytesize v1.0.2 - codeberg.org/gruf/go-byteutil v1.0.2 + codeberg.org/gruf/go-byteutil v1.1.2 codeberg.org/gruf/go-cache/v3 v3.2.2 codeberg.org/gruf/go-debug v1.3.0 codeberg.org/gruf/go-errors/v2 v2.1.1 diff --git a/go.sum b/go.sum index 203eec337..a8302697a 100644 --- a/go.sum +++ b/go.sum @@ -47,8 +47,8 @@ codeberg.org/gruf/go-bytes v1.0.2/go.mod h1:1v/ibfaosfXSZtRdW2rWaVrDXMc9E3bsi/M9 codeberg.org/gruf/go-bytesize v1.0.2 h1:Mo+ITi+0uZ4YNSZf2ed6Qw8acOI39W4mmgE1a8lslXw= codeberg.org/gruf/go-bytesize v1.0.2/go.mod h1:n/GU8HzL9f3UNp/mUKyr1qVmTlj7+xacpp0OHfkvLPs= codeberg.org/gruf/go-byteutil v1.0.0/go.mod h1:cWM3tgMCroSzqoBXUXMhvxTxYJp+TbCr6ioISRY5vSU= -codeberg.org/gruf/go-byteutil v1.0.2 h1:OesVyK5VKWeWdeDR00zRJ+Oy8hjXx1pBhn7WVvcZWVE= -codeberg.org/gruf/go-byteutil v1.0.2/go.mod h1:cWM3tgMCroSzqoBXUXMhvxTxYJp+TbCr6ioISRY5vSU= +codeberg.org/gruf/go-byteutil v1.1.2 h1:TQLZtTxTNca9xEfDIndmo7nBYxeS94nrv/9DS3Nk5Tw= +codeberg.org/gruf/go-byteutil v1.1.2/go.mod h1:cWM3tgMCroSzqoBXUXMhvxTxYJp+TbCr6ioISRY5vSU= codeberg.org/gruf/go-cache/v3 v3.2.2 h1:hq6/RITgpcArjzbYSyo3uFxfIw7wW3KqAQjEaN7dj58= codeberg.org/gruf/go-cache/v3 v3.2.2/go.mod h1:+Eje6nCvN8QF71VyYjMWMnkdv6t1kHnCO/SvyC4K12Q= codeberg.org/gruf/go-debug v1.3.0 h1:PIRxQiWUFKtGOGZFdZ3Y0pqyfI0Xr87j224IYe2snZs= diff --git a/internal/transport/deliver.go b/internal/transport/deliver.go index 476152c10..7db3bf338 100644 --- a/internal/transport/deliver.go +++ b/internal/transport/deliver.go @@ -19,7 +19,6 @@ package transport import ( - "bytes" "context" "fmt" "net/http" @@ -27,6 +26,7 @@ "strings" "sync" + "codeberg.org/gruf/go-byteutil" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/config" ) @@ -49,7 +49,7 @@ func (t *transport) BatchDeliver(ctx context.Context, b []byte, recipients []*ur wg.Wait() // receive any buffered errors - errs := make([]string, 0, len(recipients)) + errs := make([]string, 0, len(errCh)) outer: for { select { @@ -75,7 +75,11 @@ func (t *transport) Deliver(ctx context.Context, b []byte, to *url.URL) error { urlStr := to.String() - req, err := http.NewRequestWithContext(ctx, "POST", urlStr, bytes.NewReader(b)) + // Use rewindable bytes reader for body. + var body byteutil.ReadNopCloser + body.Reset(b) + + req, err := http.NewRequestWithContext(ctx, "POST", urlStr, &body) if err != nil { return err } @@ -92,7 +96,7 @@ 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 (%d): %s", urlStr, resp.StatusCode, resp.Status) + return fmt.Errorf("POST request to %s failed: %s", urlStr, resp.Status) } return nil diff --git a/internal/transport/dereference.go b/internal/transport/dereference.go index 37bdbd58f..f42f146ea 100644 --- a/internal/transport/dereference.go +++ b/internal/transport/dereference.go @@ -78,6 +78,6 @@ func (t *transport) Dereference(ctx context.Context, iri *url.URL) ([]byte, erro case http.StatusGone: return nil, ErrGone default: - return nil, fmt.Errorf("GET request to %s failed (%d): %s", iriStr, rsp.StatusCode, rsp.Status) + return nil, fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) } } diff --git a/internal/transport/derefinstance.go b/internal/transport/derefinstance.go index e46b52554..2dcf367a0 100644 --- a/internal/transport/derefinstance.go +++ b/internal/transport/derefinstance.go @@ -102,7 +102,7 @@ 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 (%d): %s", iriStr, resp.StatusCode, resp.Status) + return nil, fmt.Errorf("GET request to %s failed: %s", iriStr, resp.Status) } b, err := io.ReadAll(resp.Body) @@ -252,7 +252,7 @@ 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 (%d): %s", iriStr, resp.StatusCode, resp.Status) + return nil, fmt.Errorf("callNodeInfoWellKnown: GET request to %s failed: %s", iriStr, resp.Status) } b, err := io.ReadAll(resp.Body) @@ -303,7 +303,7 @@ 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 (%d): %s", iriStr, resp.StatusCode, resp.Status) + return nil, fmt.Errorf("callNodeInfo: GET request to %s failed: %s", iriStr, resp.Status) } b, err := io.ReadAll(resp.Body) diff --git a/internal/transport/derefmedia.go b/internal/transport/derefmedia.go index 5edfc0e44..c8a817eef 100644 --- a/internal/transport/derefmedia.go +++ b/internal/transport/derefmedia.go @@ -46,7 +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 { - return nil, 0, fmt.Errorf("GET request to %s failed (%d): %s", iriStr, rsp.StatusCode, rsp.Status) + return nil, 0, fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status) } return rsp.Body, rsp.ContentLength, nil diff --git a/internal/transport/finger.go b/internal/transport/finger.go index 1e52a59f2..4e6594df4 100644 --- a/internal/transport/finger.go +++ b/internal/transport/finger.go @@ -52,7 +52,7 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom // Check for an expected status code if rsp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("GET request to %s failed (%d): %s", urlStr, rsp.StatusCode, rsp.Status) + return nil, fmt.Errorf("GET request to %s failed: %s", urlStr, rsp.Status) } return io.ReadAll(rsp.Body) diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 18c40f79f..b0f68f707 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -32,6 +32,7 @@ "sync" "time" + "codeberg.org/gruf/go-byteutil" errorsv2 "codeberg.org/gruf/go-errors/v2" "codeberg.org/gruf/go-kv" "github.com/go-fed/httpsig" @@ -84,7 +85,7 @@ type transport struct { signerMu sync.Mutex } -// GET will perform given http request using transport client, retrying on certain preset errors, or if status code is among retryOn. +// GET will perform given http request using transport client, retrying on certain preset errors. func (t *transport) GET(r *http.Request) (*http.Response, error) { if r.Method != http.MethodGet { return nil, errors.New("must be GET request") @@ -94,7 +95,7 @@ func (t *transport) GET(r *http.Request) (*http.Response, error) { }) } -// POST will perform given http request using transport client, retrying on certain preset errors, or if status code is among retryOn. +// POST will perform given http request using transport client, retrying on certain preset errors. func (t *transport) POST(r *http.Request, body []byte) (*http.Response, error) { if r.Method != http.MethodPost { return nil, errors.New("must be POST request") @@ -116,18 +117,17 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error) (*http // Get request hostname host := r.URL.Hostname() - // Check if recently reached max retries for this host - // so we don't need to bother reattempting it. The only - // errors that are retried upon are server failure and - // domain resolution type errors, so this cached result - // indicates this server is likely having issues. - if t.controller.badHosts.Has(host) { - 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()) + if !fastFail { + // Check if recently reached max retries for this host + // so we don't bother with a retry-backoff loop. The only + // errors that are retried upon are server failure and + // domain resolution type errors, so this cached result + // indicates this server is likely having issues. + fastFail = t.controller.badHosts.Has(host) + } // Start a log entry for this request l := log.WithContext(r.Context()). @@ -148,6 +148,12 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error) (*http r.Header.Del("Signature") r.Header.Del("Digest") + // Rewind body reader and content-length if set. + if rc, ok := r.Body.(*byteutil.ReadNopCloser); ok { + r.ContentLength = int64(rc.Len()) + rc.Rewind() + } + // Perform request signing if err := signer(r); err != nil { return nil, err @@ -226,7 +232,7 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error) (*http } } - // Add "bad" entry for this host + // Add "bad" entry for this host. t.controller.badHosts.Set(host, struct{}{}) return nil, errors.New("transport reached max retries") diff --git a/vendor/codeberg.org/gruf/go-byteutil/bytes.go b/vendor/codeberg.org/gruf/go-byteutil/bytes.go index eb57a90de..227feaaa7 100644 --- a/vendor/codeberg.org/gruf/go-byteutil/bytes.go +++ b/vendor/codeberg.org/gruf/go-byteutil/bytes.go @@ -18,17 +18,20 @@ func Copy(b []byte) []byte { // B2S returns a string representation of []byte without allocation. // // According to the Go spec strings are immutable and byte slices are not. The way this gets implemented is strings under the hood are: -// type StringHeader struct { -// Data uintptr -// Len int -// } +// +// type StringHeader struct { +// Data uintptr +// Len int +// } // // while slices are: -// type SliceHeader struct { -// Data uintptr -// Len int -// Cap int -// } +// +// type SliceHeader struct { +// Data uintptr +// Len int +// Cap int +// } +// // because being mutable, you can change the data, length etc, but the string has to promise to be read-only to all who get copies of it. // // So in practice when you do a conversion of `string(byteSlice)` it actually performs an allocation because it has to copy the contents of the byte slice into a safe read-only state. @@ -54,31 +57,3 @@ func S2B(s string) []byte { return b } - -// ToUpper offers a faster ToUpper implementation using a lookup table. -func ToUpper(b []byte) { - const toUpperTable = "\x00\x01\x02\x03\x04\x05\x06\a\b\t\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" + - " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`ABCDEFGHIJKLMNOPQRSTUVWXYZ{|}~" + - "\u007f\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96" + - "\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf" + - "\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8" + - "\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1" + - "\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff" - for i := 0; i < len(b); i++ { - b[i] = toUpperTable[b[i]] - } -} - -// ToLower offers a faster ToLower implementation using a lookup table. -func ToLower(b []byte) { - const toLowerTable = "\x00\x01\x02\x03\x04\x05\x06\a\b\t\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" + - " !\"#$%&'()*+,-./0123456789:;<=>?@abcdefghijklmnopqrstuvwxyz[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" + - "\u007f\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96" + - "\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf" + - "\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8" + - "\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1" + - "\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff" - for i := 0; i < len(b); i++ { - b[i] = toLowerTable[b[i]] - } -} diff --git a/vendor/codeberg.org/gruf/go-byteutil/reader.go b/vendor/codeberg.org/gruf/go-byteutil/reader.go new file mode 100644 index 000000000..94c755ff4 --- /dev/null +++ b/vendor/codeberg.org/gruf/go-byteutil/reader.go @@ -0,0 +1,36 @@ +package byteutil + +import "bytes" + +// Reader wraps a bytes.Reader{} to provide Rewind() capabilities. +type Reader struct { + B []byte + bytes.Reader +} + +// NewReader returns a new Reader{} instance reset to b. +func NewReader(b []byte) *Reader { + r := &Reader{} + r.Reset(b) + return r +} + +// Reset resets the Reader{} to be reading from b and sets Reader{}.B. +func (r *Reader) Reset(b []byte) { + r.B = b + r.Rewind() +} + +// Rewind resets the Reader{} to be reading from the start of Reader{}.B. +func (r *Reader) Rewind() { + r.Reader.Reset(r.B) +} + +// ReadNopCloser wraps a Reader{} to provide nop close method. +type ReadNopCloser struct { + Reader +} + +func (*ReadNopCloser) Close() error { + return nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index bc8da6918..99814c157 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -10,7 +10,7 @@ codeberg.org/gruf/go-bytes # codeberg.org/gruf/go-bytesize v1.0.2 ## explicit; go 1.17 codeberg.org/gruf/go-bytesize -# codeberg.org/gruf/go-byteutil v1.0.2 +# codeberg.org/gruf/go-byteutil v1.1.2 ## explicit; go 1.16 codeberg.org/gruf/go-byteutil # codeberg.org/gruf/go-cache/v3 v3.2.2