refactor!: remove response bytes method in-favor Body field, cleanup,…

… apply appropriate naming struct, methods (#871)

client.go

@@ -51,6 +51,9 @@ const (
 )
 
 var (
+	ErrNotHttpTransportType       = errors.New("resty: not a http.Transport type")
+	ErrUnsupportedRequestBodyKind = errors.New("resty: unsupported request body kind")
+
 	hdrUserAgentKey       = http.CanonicalHeaderKey("User-Agent")
 	hdrAcceptKey          = http.CanonicalHeaderKey("Accept")
 	hdrAcceptEncodingKey  = http.CanonicalHeaderKey("Accept-Encoding")
@@ -1383,8 +1386,6 @@ func (c *Client) SetRateLimiter(rl RateLimiter) *Client {
 	return c
 }
 
-var ErrNotHttpTransportType = errors.New("resty: not a http.Transport type")
-
 // Transport method returns [http.Transport] currently in use or error
 // in case the currently used `transport` is not a [http.Transport].
 //
@@ -1816,9 +1817,9 @@ func (c *Client) execute(req *Request) (*Response, error) {
 		response.wrapLimitReadCloser()
 	}
 	if !req.DoNotParseResponse && (req.Debug || req.ResponseBodyUnlimitedReads) {
-		response.wrapReadCopier()
+		response.wrapCopyReadCloser()
 
-		if err := response.readAllBytes(); err != nil {
+		if err := response.readAll(); err != nil {
 			return response, err
 		}
 	}

client_test.go

@@ -192,8 +192,8 @@ func TestClientRedirectPolicy(t *testing.T) {
 
 	c.SetRedirectPolicy(NoRedirectPolicy())
 	_, err = c.R().Get(ts.URL + "/redirect-1")
-	assertEqual(t, true, (err.Error() == "Get /redirect-2: auto redirect is disabled" ||
-		err.Error() == "Get \"/redirect-2\": auto redirect is disabled"))
+	assertEqual(t, true, (err.Error() == "Get /redirect-2: resty: auto redirect is disabled" ||
+		err.Error() == "Get \"/redirect-2\": resty: auto redirect is disabled"))
 }
 
 func TestClientTimeout(t *testing.T) {
@@ -380,7 +380,6 @@ func TestClientOnBeforeRequestModification(t *testing.T) {
 	assertError(t, err)
 	assertEqual(t, http.StatusOK, resp.StatusCode())
 	assertEqual(t, "200 OK", resp.Status())
-	assertNotNil(t, resp.BodyBytes())
 	assertEqual(t, "TestGet: text response", resp.String())
 
 	logResponse(t, resp)
@@ -481,6 +480,13 @@ func TestClientSettingsCoverage(t *testing.T) {
 
 	ct.outputLogTo(io.Discard)
 	// [End] Custom Transport scenario
+
+	// Response - for now stay here
+	resp := &Response{Request: &Request{}}
+	s, err := resp.fmtBodyString(0)
+	assertNil(t, err)
+	assertEqual(t, "***** NO CONTENT *****", s)
+	fmt.Println(err, s)
 }
 
 func TestContentLengthWhenBodyIsNil(t *testing.T) {
@@ -533,6 +539,21 @@ func TestClientPreRequestHook(t *testing.T) {
 	assertEqual(t, `{ "id": "success", "message": "login successful" }`, resp.String())
 }
 
+func TestClientPreRequestHookError(t *testing.T) {
+	ts := createGetServer(t)
+	defer ts.Close()
+
+	c := dcnl()
+	c.SetPreRequestHook(func(c *Client, r *http.Request) error {
+		return errors.New("error from PreRequestHook")
+	})
+
+	resp, err := c.R().Get(ts.URL)
+	assertNotNil(t, err)
+	assertEqual(t, "error from PreRequestHook", err.Error())
+	assertNil(t, resp)
+}
+
 func TestClientAllowsGetMethodPayload(t *testing.T) {
 	ts := createGetServer(t)
 	defer ts.Close()
@@ -661,7 +682,6 @@ func TestGzipCompress(t *testing.T) {
 		assertError(t, err)
 		assertEqual(t, http.StatusOK, resp.StatusCode())
 		assertEqual(t, "200 OK", resp.Status())
-		assertNotNil(t, resp.BodyBytes())
 		assertEqual(t, tc.want, resp.String())
 
 		logResponse(t, resp)
@@ -684,7 +704,6 @@ func TestDeflateCompress(t *testing.T) {
 		assertError(t, err)
 		assertEqual(t, http.StatusOK, resp.StatusCode())
 		assertEqual(t, "200 OK", resp.Status())
-		assertNotNil(t, resp.BodyBytes())
 		assertEqual(t, tc.want, resp.String())
 
 		logResponse(t, resp)
@@ -738,7 +757,6 @@ func TestLzwCompress(t *testing.T) {
 		assertError(t, err)
 		assertEqual(t, http.StatusOK, resp.StatusCode())
 		assertEqual(t, "200 OK", resp.Status())
-		assertNotNil(t, resp.BodyBytes())
 		assertEqual(t, tc.want, resp.String())
 
 		logResponse(t, resp)
@@ -1216,7 +1234,7 @@ func TestResponseBodyLimit(t *testing.T) {
 
 		res, err := c.R().SetResponseBodyLimit(800*100 + 10).Get(ts.URL + "/")
 		assertNil(t, err)
-		assertEqual(t, 800*100, len(res.BodyBytes()))
+		assertEqual(t, 800*100, len(res.bodyBytes))
 		assertEqual(t, int64(800*100), res.Size())
 	})
 
@@ -1225,7 +1243,7 @@ func TestResponseBodyLimit(t *testing.T) {
 
 		res, err := c.R().Get(ts.URL + "/")
 		assertNil(t, err)
-		assertEqual(t, 800*100, len(res.BodyBytes()))
+		assertEqual(t, 800*100, len(res.bodyBytes))
 		assertEqual(t, int64(800*100), res.Size())
 	})
 

context_test.go

@@ -27,7 +27,6 @@ func TestSetContext(t *testing.T) {
 	assertError(t, err)
 	assertEqual(t, http.StatusOK, resp.StatusCode())
 	assertEqual(t, "200 OK", resp.Status())
-	assertEqual(t, true, resp.BodyBytes() != nil)
 	assertEqual(t, "TestGet: text response", resp.String())
 
 	logResponse(t, resp)

middleware.go

@@ -6,7 +6,6 @@ package resty
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"mime/multipart"
@@ -166,8 +165,11 @@ func parseRequestHeader(c *Client, r *Request) error {
 		r.Header.Set(hdrUserAgentKey, hdrUserAgentValue)
 	}
 
-	if ct := r.Header.Get(hdrContentTypeKey); isStringEmpty(r.Header.Get(hdrAcceptKey)) && !isStringEmpty(ct) && (isJSONContentType(ct) || isXMLContentType(ct)) {
-		r.Header.Set(hdrAcceptKey, r.Header.Get(hdrContentTypeKey))
+	if isStringEmpty(r.Header.Get(hdrAcceptKey)) {
+		ct := r.Header.Get(hdrContentTypeKey)
+		if isJSONContentType(ct) || isXMLContentType(ct) {
+			r.Header.Set(hdrAcceptKey, ct)
+		}
 	}
 
 	if isStringEmpty(r.Header.Get(hdrAcceptEncodingKey)) {
@@ -390,8 +392,6 @@ func parseResponseBody(c *Client, res *Response) (err error) {
 		return
 	}
 
-	// TODO Attention Required when working on Compression
-
 	rct := firstNonEmpty(
 		res.Request.ForceResponseContentType,
 		res.Header().Get(hdrContentTypeKey),
@@ -401,7 +401,7 @@ func parseResponseBody(c *Client, res *Response) (err error) {
 	decFunc, found := c.inferContentTypeDecoder(rct, decKey)
 	if !found {
 		// the Content-Type decoder is not found; just read all the body bytes
-		err = res.readAllBytes()
+		err = res.readAll()
 		return
 	}
 
@@ -430,7 +430,7 @@ func parseResponseBody(c *Client, res *Response) (err error) {
 	}
 
 	// read all bytes when auto-unmarshal didn't take place
-	err = res.readAllBytes()
+	err = res.readAll()
 	return
 }
 
@@ -499,8 +499,6 @@ func handleFormData(c *Client, r *Request) {
 	r.isFormData = true
 }
 
-var ErrUnsupportedRequestBodyKind = errors.New("resty: unsupported request body kind")
-
 func handleRequestBody(c *Client, r *Request) error {
 	contentType := r.Header.Get(hdrContentTypeKey)
 	if isStringEmpty(contentType) {
@@ -513,7 +511,7 @@ func handleRequestBody(c *Client, r *Request) error {
 
 	switch body := r.Body.(type) {
 	case io.Reader:
-		// TODO create pass through reader to capture content-length
+		// TODO create pass through reader to capture content-length, really needed??
 		if r.setContentLength { // keep backward compatibility
 			if _, err := r.bodyBuf.ReadFrom(body); err != nil {
 				releaseBuffer(r.bodyBuf)

redirect.go

@@ -12,9 +12,7 @@ import (
 	"strings"
 )
 
-var (
-	ErrAutoRedirectDisabled = errors.New("auto redirect is disabled")
-)
+var ErrAutoRedirectDisabled = errors.New("resty: auto redirect is disabled")
 
 type (
 	// RedirectPolicy to regulate the redirects in the Resty client.
@@ -23,7 +21,7 @@ type (
 	// Apply function should return nil to continue the redirect journey; otherwise
 	// return error to stop the redirect.
 	RedirectPolicy interface {
-		Apply(req *http.Request, via []*http.Request) error
+		Apply(*http.Request, []*http.Request) error
 	}
 
 	// The [RedirectPolicyFunc] type is an adapter to allow the use of ordinary

request.go

@@ -89,17 +89,12 @@ func (r *Request) GenerateCurlCommand() string {
 	if !(r.Debug && r.generateCurlOnDebug) {
 		return ""
 	}
-
 	if r.resultCurlCmd != nil {
 		return *r.resultCurlCmd
 	}
-
 	if r.RawRequest == nil {
 		r.client.executeBefore(r) // mock with r.Get("/")
 	}
-	if r.resultCurlCmd == nil {
-		r.resultCurlCmd = new(string)
-	}
 	*r.resultCurlCmd = buildCurlRequest(r)
 	return *r.resultCurlCmd
 }

request_test.go

@@ -44,7 +44,6 @@ func TestGet(t *testing.T) {
 	assertEqual(t, http.StatusOK, resp.StatusCode())
 	assertEqual(t, "HTTP/1.1", resp.Proto())
 	assertEqual(t, "200 OK", resp.Status())
-	assertNotNil(t, resp.BodyBytes())
 	assertEqual(t, "TestGet: text response", resp.String())
 
 	logResponse(t, resp)
@@ -1873,7 +1872,6 @@ func TestRequestQueryStringOrder(t *testing.T) {
 	assertError(t, err)
 	assertEqual(t, http.StatusOK, resp.StatusCode())
 	assertEqual(t, "200 OK", resp.Status())
-	assertNotNil(t, resp.BodyBytes())
 	assertEqual(t, "TestGet: text response", resp.String())
 
 	logResponse(t, resp)
@@ -1937,7 +1935,6 @@ func TestHostHeaderOverride(t *testing.T) {
 	assertError(t, err)
 	assertEqual(t, http.StatusOK, resp.StatusCode())
 	assertEqual(t, "200 OK", resp.Status())
-	assertNotNil(t, resp.BodyBytes())
 	assertEqual(t, "myhostname", resp.String())
 
 	logResponse(t, resp)
@@ -2392,4 +2389,8 @@ func TestRequestSettingsCoverage(t *testing.T) {
 	c.R().SetResponseBodyUnlimitedReads(true)
 
 	c.R().DisableDebug()
+
+	invalidJsonBytes := []byte(`{\" \": "value here"}`)
+	result := jsonIndent(invalidJsonBytes)
+	assertEqual(t, string(invalidJsonBytes), string(result))
 }

response.go

@@ -30,21 +30,6 @@ type Response struct {
 	receivedAt time.Time
 }
 
-// BodyBytes method returns the HTTP response as `[]byte` slice for the executed request.
-//
-// NOTE:
-//   - [Response.BodyBytes] might be `nil` if [Request.SetOutputFile], [Request.SetDoNotParseResponse],
-//     [Client.SetDoNotParseResponse] method is used.
-//   - [Response.BodyBytes] might be `nil` if [Response].Body is already auto-unmarshal performed.
-//
-// TODO remove it
-func (r *Response) BodyBytes() []byte {
-	if r.RawResponse == nil {
-		return []byte{}
-	}
-	return r.bodyBytes
-}
-
 // Status method returns the HTTP status string for the executed request.
 //
 //	Example: 200 OK
@@ -107,10 +92,12 @@ func (r *Response) Cookies() []*http.Cookie {
 // It returns an empty string if it is nil or the body is zero length.
 //
 // NOTE:
-//   - Returns an empty string on auto-unmarshal scenarios
+//   - Returns an empty string on auto-unmarshal scenarios, unless
+//     [Client.SetResponseBodyUnlimitedReads] or [Request.SetResponseBodyUnlimitedReads] set.
+//   - Returns an empty string when [Client.SetDoNotParseResponse] or [Request.SetDoNotParseResponse] set
 func (r *Response) String() string {
 	if len(r.bodyBytes) == 0 && !r.Request.DoNotParseResponse {
-		_ = r.readAllBytes()
+		_ = r.readAll()
 	}
 	return strings.TrimSpace(string(r.bodyBytes))
 }
@@ -191,17 +178,17 @@ func (r *Response) fmtBodyString(sl int) (string, error) {
 
 // auto-unmarshal didn't happen, so fallback to
 // old behavior of reading response as body bytes
-func (r *Response) readAllBytes() (err error) {
+func (r *Response) readAll() (err error) {
 	if r.Body == nil || r.IsRead {
 		return nil
 	}
 
-	if _, ok := r.Body.(*readCopier); ok {
+	if _, ok := r.Body.(*copyReadCloser); ok {
 		_, err = io.ReadAll(r.Body)
 	} else {
 		r.bodyBytes, err = io.ReadAll(r.Body)
 		closeq(r.Body)
-		r.Body = &readNoOpCloser{r: bytes.NewReader(r.bodyBytes)}
+		r.Body = &nopReadCloser{r: bytes.NewReader(r.bodyBytes)}
 	}
 	if err == io.ErrUnexpectedEOF {
 		// content-encoding scenario's - empty/no response body from server
@@ -222,14 +209,14 @@ func (r *Response) wrapLimitReadCloser() {
 	}
 }
 
-func (r *Response) wrapReadCopier() {
-	r.Body = &readCopier{
+func (r *Response) wrapCopyReadCloser() {
+	r.Body = &copyReadCloser{
 		s: r.Body,
 		t: acquireBuffer(),
 		f: func(b *bytes.Buffer) {
 			r.bodyBytes = append([]byte{}, b.Bytes()...)
 			closeq(r.Body)
-			r.Body = &readNoOpCloser{r: bytes.NewReader(r.bodyBytes)}
+			r.Body = &nopReadCloser{r: bytes.NewReader(r.bodyBytes)}
 			releaseBuffer(b)
 		},
 	}