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

panic("todo") when marshalling cbor tags #9

Closed
dignifiedquire opened this issue Nov 13, 2017 · 5 comments
Closed

panic("todo") when marshalling cbor tags #9

dignifiedquire opened this issue Nov 13, 2017 · 5 comments

Comments

@dignifiedquire
Copy link
Contributor

Hey,

I am trying to use the new tag functionality discussed in #7. But I have run into a beautiful panic todo.

You can see the trace below.

--- FAIL: TestBasicMarshal (0.00s)
panic: todo [recovered]
	panic: todo

goroutine 8 [running]:
testing.tRunner.func1(0xc4200b23c0)
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:711 +0x2d2
panic(0x11b56e0, 0x121abe0)
	/usr/local/Cellar/go/1.9.1/libexec/src/runtime/panic.go:491 +0x283
github.com/polydawn/refmt/obj._yieldMarshalMachinePtr(0xc4200c52d0, 0xc420076390, 0xc4200763c0, 0x12e8a60, 0x11d88e0, 0x10, 0x11c2a60)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/obj/marshalSlab.go:141 +0x718
github.com/polydawn/refmt/obj.(*marshalSlab).requisitionMachine(0xc420082a50, 0x12e8a60, 0x11d88e0, 0x12e8a60, 0x11e74a0)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/obj/marshalSlab.go:60 +0x179
github.com/polydawn/refmt/obj.(*marshalMachineWildcard).Reset(0xc4200c51a8, 0xc420082a50, 0x11c2a60, 0xc42004caf0, 0x94, 0x12e8a60, 0x11c2a60, 0x11b56e0, 0xc42004cae0)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/obj/marshalWildcard.go:26 +0xe7
github.com/polydawn/refmt/obj.(*Marshaller).Recurse(0xc420082a50, 0xc4200b6d20, 0x11c2a60, 0xc42004caf0, 0x94, 0x12e8a60, 0x11c2a60, 0x12e45a0, 0xc4200c51a8, 0x1365000, ...)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/obj/marshal.go:100 +0xe2
github.com/polydawn/refmt/obj.(*marshalMachineMapWildcard).Step(0xc4200c5050, 0xc420082a50, 0xc420082a50, 0xc4200b6d20, 0x0, 0x0, 0x0)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/obj/marshalMapWildcard.go:78 +0x1e6
github.com/polydawn/refmt/obj.(*Marshaller).Step(0xc420082a50, 0xc4200b6d20, 0x0, 0x0, 0x0)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/obj/marshal.go:58 +0x50
github.com/polydawn/refmt/shared.TokenPump.Run(0x12e3720, 0xc420082a50, 0x12e36a0, 0xc420082aa0, 0x0, 0xc4200c5000)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/shared/pump.go:31 +0x5b
github.com/polydawn/refmt/cbor.(*Marshaller).Marshal(0xc420076a80, 0x11c6aa0, 0xc420076a50, 0xc4200763c0, 0xc420076a80)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/cbor/cborHelpers.go:54 +0x83
github.com/polydawn/refmt/cbor.MarshalAtlased(0x11c6aa0, 0xc420076a50, 0xc420076390, 0xc4200763c0, 0x10b6c81, 0xc4200a0000, 0xc420010700, 0x34, 0x40)
	/Users/dignifiedquire/.go/src/github.com/polydawn/refmt/cbor/cborHelpers.go:39 +0x81
github.com/ipfs/go-ipld-cbor.DumpObject(0x11c6aa0, 0xc420076a50, 0xc4200a0000, 0x0, 0x0, 0xc420048e08, 0x10b6d32)
	/Users/dignifiedquire/.go/src/github.com/ipfs/go-ipld-cbor/node.go:424 +0x51
github.com/ipfs/go-ipld-cbor.WrapObject(0x11c6aa0, 0xc420076a50, 0x12, 0xffffffffffffffff, 0x1, 0x34, 0x0)
	/Users/dignifiedquire/.go/src/github.com/ipfs/go-ipld-cbor/node.go:165 +0x4d
github.com/ipfs/go-ipld-cbor.TestBasicMarshal(0xc4200b23c0)
	/Users/dignifiedquire/.go/src/github.com/ipfs/go-ipld-cbor/node_test.go:84 +0x2a3
testing.tRunner(0xc4200b23c0, 0x12043f0)
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:789 +0x2de

The

From what I gathered this seems to come from here: https://github.com/polydawn/refmt/blob/master/obj/marshalSlab.go#L141 but I am not sure if this is something I should fix in my code or this needs to be fixed in the library.

@warpfork
Copy link
Member

It is indeed a TODO for refmt :)

This path is hit when marshalling traverses to struct value of a type which hasn't had explicit configuration in the atlas. If you add another atlas.BuildEntry() /*...*/ for whatever type is missing, you'll be back on the happy path.

The future plan is to make additional options to the overall atlas build for whether or not to silently try to autogenerate configuration for structs when they're encountered. This would be equivalent to what the stdlib json packages do. In refmt I think we'll probably keep that to off by default, based on the theology that explicit is better than implicit (and personally, mostly because I'm really frustrated whenever I find an error from fmt.Errorf has been quietly serialized to a useless json {}), but the option should be supported.

Of course, the way this currently errors is more aggressively than it should, and the fact that the message doesn't really point you in the right direction is a pretty gross bug, and I'll try to at least improve those messages ASAP.

warpfork added a commit that referenced this issue Nov 13, 2017
Slightly improves the situation in issue #9.

Signed-off-by: Eric Myhre <hash@exultant.us>
@warpfork
Copy link
Member

Error message should be more useful as of that commit on master ^

@dignifiedquire
Copy link
Contributor Author

Thank you 👍

Interestingly enough the error went away when I switched to using the non pointer version of my struct, without changing anything else in the atlas.

@warpfork
Copy link
Member

Ohhh. Hm, that's a thing that should have its own checks. The object used as the "type reference" in atlas.BuildEntry(thingy{}) is expected to be a bare object, yes. I opened #11 to remind myself of this in the morning (getting a bit late here).

@warpfork
Copy link
Member

GC/closing because #11 covers it.

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

No branches or pull requests

2 participants