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: handling of nil passed as an arg to a typed interface parameter … #161

Open
wants to merge 3 commits into
base: v3
Choose a base branch
from

Conversation

shaunco
Copy link
Contributor

@shaunco shaunco commented Nov 20, 2021

…for #160

@vmihailenco
Copy link
Owner

Please add a test.

@shaunco
Copy link
Contributor Author

shaunco commented Nov 26, 2021

Turns out the code path that I fixed was only used for messages that are never serialized with msgpack (such as memqueue). In writing the new unit test, I'm having very strange issues on the Redis side where msgpack is used:

Exception has occurred: panic
reflect.Set: value of type map[string]interface {} is not assignable to type taskq_test.testInterface
Stack:
	 2  0x0000000000494f91 in reflect.Value.assignTo
	     at /usr/local/go/src/reflect/value.go:2797
	 3  0x00000000004917f2 in reflect.Value.Set
	     at /usr/local/go/src/reflect/value.go:1905
	 4  0x00000000006f744f in github.com/vmihailenco/msgpack/v5.(*Decoder).interfaceValue
	     at /usr/local/go/pkg/mod/github.com/vmihailenco/msgpack/[email protected]/decode_value.go:201
	 5  0x00000000006f709b in github.com/vmihailenco/msgpack/v5.decodeInterfaceValue
	     at /usr/local/go/pkg/mod/github.com/vmihailenco/msgpack/[email protected]/decode_value.go:182
	 6  0x00000000006e8702 in github.com/vmihailenco/msgpack/v5.(*Decoder).DecodeValue
	     at /usr/local/go/pkg/mod/github.com/vmihailenco/msgpack/[email protected]/decode.go:309
	 7  0x00000000007489df in github.com/vmihailenco/taskq/v3.(*reflectFunc).fnArgs
	     at /dev/taskq-working/handler.go:147
	 8  0x000000000074804f in github.com/vmihailenco/taskq/v3.(*reflectFunc).HandleMessage
	     at /dev/taskq-working/handler.go:73
	 9  0x000000000074c61a in github.com/vmihailenco/taskq/v3.(*Task).HandleMessage
	     at /dev/taskq-working/task.go:112
	10  0x000000000074ba2a in github.com/vmihailenco/taskq/v3.(*TaskMap).HandleMessage
	     at /dev/taskq-working/registry.go:71
	11  0x0000000000742b7f in github.com/vmihailenco/taskq/v3.(*Consumer).Process
	     at /dev/taskq-working/consumer.go:539
	12  0x000000000074250d in github.com/vmihailenco/taskq/v3.(*Consumer).worker
	     at /dev/taskq-working/consumer.go:480
	13  0x0000000000741098 in github.com/vmihailenco/taskq/v3.(*Consumer).addWorker.func1
	     at /dev/taskq-working/consumer.go:276

msgpack seems to encode/decode nil interfaces just fine, but when I pass in an actual object as an interface the decode seems to think the interface object is a map[string]interface{}.

I don't know msgpack well enough to have a solution yet, but am still digging.

@shaunco
Copy link
Contributor Author

shaunco commented Nov 26, 2021

Ok, ignore the above... After reading vmihailenco/msgpack#261 , I see my understanding of msgpack was flawed.

Since the code path I patched is just for memqueue (where args are not msgpacked), I put the test in memqueue_test.go.

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.

2 participants