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

OnBeforeRequest does not change Request.RawRequest attributes #57

Closed
ChristianLohmann opened this issue Feb 24, 2017 · 8 comments
Closed
Assignees
Labels

Comments

@ChristianLohmann
Copy link

Trying to manipulate *resty.Request fields inside an OnBeforeRequest function does not affect the actual request, which is finally executed:

https://github.com/go-resty/resty/blob/master/client.go#L627-L645

for _, f := range c.beforeRequest {
		err = f(c, req)
		if err != nil {
			return nil, err
		}
	}

	c.mutex.Lock()

	if req.proxyURL != nil {
		c.transport.Proxy = http.ProxyURL(req.proxyURL)
	} else if c.proxyURL != nil {
		c.transport.Proxy = http.ProxyURL(c.proxyURL)
	}

	req.Time = time.Now()
	c.httpClient.Transport = c.transport

	resp, err := c.httpClient.Do(req.RawRequest)

All OnBeforeRequest functions will be executed but they have no effect when they call methods directly on *resty.Request because the RawRequest stays untouched until real request execution.

For example setting an auth token header inside an OnBeforeRequest function won't work with

	httpClient.OnBeforeRequest(func(c *resty.Client, r *resty.Request) error {
		r.SetAuthToken("bla")
                return nil
        })

As a workaround one has to manipulate directly the r.RawRequest struct.

@jeevatkm jeevatkm self-assigned this Feb 24, 2017
@jeevatkm
Copy link
Member

Thanks for reporting @ChristianLohmann. I will have a look.

@jeevatkm jeevatkm added this to the v0.11 Milestone milestone Feb 24, 2017
@jeevatkm jeevatkm added the bug label Feb 24, 2017
@jeevatkm
Copy link
Member

I had a look and understood what you have describe. I will correct the execution sequence of OnBeforeRequest.

Please continue to modify the *resty.Request after the fix. Tomorrow fix will be available on master and will be released part of v0.11.

@ChristianLohmann
Copy link
Author

Many thanks for this quick reply and providing a fix that fast!

jeevatkm added a commit that referenced this issue Feb 24, 2017
@jeevatkm
Copy link
Member

Fix available on master branch. Please give it a try and let me know.

@ChristianLohmann
Copy link
Author

Thanks, it works now!

@jeevatkm
Copy link
Member

Thanks for the confirmation. I believe master branch is working without any issues.

@ChristianLohmann
Copy link
Author

I haven't experienced any other issues with the master so far. I glued the commit hash to c53a09a with glide until a new release is available via gopkg.in.

@jeevatkm
Copy link
Member

Thank you. I'm closing this issue.

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

2 participants