Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execute user defined middlewares after resty's internal middlewares produced backward incompatible changes #408

Closed
buglloc opened this issue Feb 1, 2021 · 5 comments
Assignees
Labels

Comments

@buglloc
Copy link
Contributor

buglloc commented Feb 1, 2021

Latest v2.4.0 release have backward incompatible changes: #355
In this regard, I want to clarify - that now we can't manipulate the final request through *resty.Request is expected behavior or not? And now we need to rewrite all of our OnBeforeRequest middlewares?

Reproducer:

package main

import (
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"
	"os"

	"github.com/go-resty/resty/v2"
)

func main() {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		_, _ = io.Copy(os.Stdout, r.Body)
		_, _ = fmt.Fprintln(w, "LGTM")
	}))
	defer srv.Close()

	_, _ = resty.New().
		OnBeforeRequest(func(_ *resty.Client, r *resty.Request) error {
			r.Body = []int{1, 2, 3}
			return nil
		}).R().
		SetBody([]int{0, 0, 0}).
		Post(srv.URL)
}

Before v2.4.0 (this behavior introduced in the #57 afaik) output:

[1,2,3]

v2.4.0:

[0,0,0]
@moorereason
Copy link
Contributor

I agree. We should not have merged this breaking change into a minor release.

My recommendation would be to revert the change and issue a new minor release (v.2.5.0) ASAP. Then we can open a new issue and discuss the problem #355 was trying to solve.

Cc: @jeevatkm

@jeevatkm
Copy link
Member

jeevatkm commented Feb 7, 2021

@buglloc I'm sorry for the delay. I agree with @moorereason, apologies for the inconvenience. I will try to revert and make a release tonight or tomorrow. We will decide later on what do to with revert change.

@buglloc
Copy link
Contributor Author

buglloc commented Feb 10, 2021

@jeevatkm any news? :)
The longer we wait with reverting, the more people can starts using the new behavior :(

@jeevatkm
Copy link
Member

@buglloc @moorereason Revert has been done!

@buglloc
Copy link
Contributor Author

buglloc commented Feb 11, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants