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

Fix error interface #42

Merged
merged 3 commits into from Apr 23, 2023
Merged

Fix error interface #42

merged 3 commits into from Apr 23, 2023

Conversation

vazir
Copy link
Contributor

@vazir vazir commented Apr 22, 2023

Current implementation leads to following panic, it's a fix.

panic: errors: *target must be interface or implement error

goroutine 11 [running]:
errors.As(0x7644e0, 0xc000222090, 0x6ba480, 0xc00021e108, 0xc0001c0000)
/usr/lib/go-1.15/src/errors/wrap.go:87 +0x533
github.com/pkg/errors.As(...)
/root/go/pkg/mod/github.com/pkg/errors@v0.9.1/go113.go:31
github.com/wmnsk/go-m3ua.(*Conn).handleErrors(0xc000058240, 0x7644e0, 0xc000222090, 0x1, 0xc000056101)
/opt/go-m3ua/errors.go:146 +0x73
github.com/wmnsk/go-m3ua.(*Conn).monitor(0xc000058240, 0x7692c0, 0xc000056140)
/opt/go-m3ua/fsm.go:196 +0x525
created by github.com/wmnsk/go-m3ua.Dial
/opt/go-m3ua/client.go:49 +0x235
exit status 2

Copy link
Owner

@wmnsk wmnsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, thanks! Could you update all with my suggestion to ErrInvalidVersion? It includes two points;

  • Please don't use variables starting with _.
  • No need to instantiate like &ErrInvalidVersion{}. You can just declare with pointer and refer to it.

errors.go Outdated
Comment on lines 146 to 147
_ErrInvalidVersion := &ErrInvalidVersion{}
if errors.As(e, &_ErrInvalidVersion) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_ErrInvalidVersion := &ErrInvalidVersion{}
if errors.As(e, &_ErrInvalidVersion) {
var errInvalidVersion *ErrInvalidVersion
if errors.As(e, &errInvalidVersion) {

@vazir
Copy link
Contributor Author

vazir commented Apr 23, 2023

Did as you suggested, please take a look

Copy link
Owner

@wmnsk wmnsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfect, thanks!

@wmnsk wmnsk merged commit c349603 into wmnsk:master Apr 23, 2023
3 checks passed
@vazir vazir deleted the fix-error-interface branch April 24, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants