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

Interface Panic #651

Closed
hoc-tiki opened this issue May 4, 2023 · 5 comments · Fixed by #693
Closed

Interface Panic #651

hoc-tiki opened this issue May 4, 2023 · 5 comments · Fixed by #693
Assignees

Comments

@hoc-tiki
Copy link

hoc-tiki commented May 4, 2023

receiving a response (I don't know what exactly response from third party), and I've got a panic
So what should I need to avoid this panic. Thank you

Code

resty.
		SetDebug(false).
		SetTimeout(1*time.Minute).
		R().
		SetBody(param).
		SetHeader("authorization", <api token>).
		SetResult(&response).
		SetError(&responseErr).
		Post(v1_pathInternalAsaDiscountRefund)

Error

panic: interface conversion: interface {} is nil, not string
goroutine 2463 [running]:
github.com/go-resty/resty/v2.responseLogger(0xc0001ae960, 0xc00b441d10)
/go/pkg/mod/github.com/go-resty/resty/[email protected]/middleware.go:301 +0x75c
github.com/go-resty/resty/v2.(*Client).execute(0xc0001ae960, 0xc00b4511e0)
/go/pkg/mod/github.com/go-resty/resty/[email protected]/client.go:972 +0xbe4
github.com/go-resty/resty/v2. (*Request).Execute(0xc00b4511e0,
{0x1150590?, 0xc00b4d95a0?}, {0x117ec20, 0x21})
/go/pkg/mod/github.com/go-resty/resty/[email protected]/request.go:758 +0x5c5 github.com/go-resty/resty/v2. (*Request). Post(...)
/go/pkg/mod/github.com/go-resty/resty/[email protected]/request.go:699
@Greyh4t
Copy link

Greyh4t commented Aug 9, 2023

This problem can be avoided by calling SetDebug once when *resty.Client is initialized globally.
Otherwise, if you share the same *resty.Client, and SetDebug and send requests concurrently, you may encounter this problem.
I think this is a bug in resty, because SetDebug is not concurrently safe. When SetDebug is called concurrently, Request.values may be empty. At this time, the responseLogger method will definitely panic when fetching data from it.
See also:
https://github.com/go-resty/resty/blob/master/middleware.go#L281
https://github.com/go-resty/resty/blob/master/middleware.go#L300

This is a trigger example:

var wg sync.WaitGroup
c := resty.New()
for i := 0; i < 10; i++ {
	wg.Add(1)
	go func(i int) {
		if i%2 == 0 {
			c.SetDebug(true)
		} else {
			c.SetDebug(false)
		}
		_, err := c.R().Get("https://www.feishu.cn")
		fmt.Println(err)
		wg.Done()
	}(i)
}
wg.Wait()

@hoc-tiki
Copy link
Author

Thank for your answer, but why you don't return an error if it's empty. This error makes crashing app

@Greyh4t
Copy link

Greyh4t commented Aug 10, 2023

Thank for your answer, but why you don't return an error if it's empty. This error makes crashing app

I am not a developer of resty, I encountered the same problem as you, the reason for this problem is that resty encountered an unexpected assertion, resty needs to fix it

@jeevatkm
Copy link
Member

@Greyh4t Thanks for the analysis and sample test case.

@hoc-tiki @Greyh4t I will look into this.

@jeevatkm
Copy link
Member

@Greyh4t @hoc-tiki I have looked at it. Having too many RWMutex is not good, so I'm going to add the SetDebug method at the request level.

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

Successfully merging a pull request may close this issue.

3 participants