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

codecs: error upon encountering an 'absent' value during encodes. #196

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ Unreleased on master
Changes here are on the master branch, but not in any tagged release yet.
When a release tag is made, this block of bullet points will just slide down to the [Released Changes](#released-changes) section.

- _nothing yet :)_
- Changed: encoding functions for codecs in this repo (dag-cbor, dag-json, cbor, json) will now **error** if they encounter an `ipld.Absent` node (or more specifically, if `IsAbsent() == true`).
[[#196](https://github.com/ipld/go-ipld-prime/pull/196)]
- Previously, these codecs would quietly emit a serial "null" token in this scenario -- which was effectively implicitly coercing a value into null. Implicit coercion creates bugs and is bad, so we will not be doing it anymore.
- This situation is only reachable when working with schema-typed data (since otherwise `Absent` values aren't used). In that situation, handing that typed value to an encode function without having called `.Representation()` first is also typically a logical error, so, returning an explicit error now is also probably the right thing to do.



Expand Down
5 changes: 4 additions & 1 deletion codec/dagcbor/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ func Marshal(n ipld.Node, sink shared.TokenSink, allowLinks bool) error {
func marshal(n ipld.Node, tk *tok.Token, sink shared.TokenSink, allowLinks bool) error {
switch n.Kind() {
case ipld.Kind_Invalid:
return fmt.Errorf("cannot traverse a node that is absent")
return fmt.Errorf("invalid node encountered")
case ipld.Kind_Null:
if n.IsAbsent() {
return fmt.Errorf("cannot encode a node that is absent")
}
tk.Type = tok.TNull
_, err := sink.Step(tk)
return err
Expand Down
5 changes: 4 additions & 1 deletion codec/dagjson/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ func Marshal(n ipld.Node, sink shared.TokenSink, allowLinks bool) error {
var tk tok.Token
switch n.Kind() {
case ipld.Kind_Invalid:
return fmt.Errorf("cannot traverse a node that is absent")
return fmt.Errorf("invalid node encountered")
case ipld.Kind_Null:
if n.IsAbsent() {
return fmt.Errorf("cannot encode a node that is absent")
Copy link
Member

Choose a reason for hiding this comment

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

i worry that this error message, which will be at runtime, will be fairly opaque. Consider adding a hint like "perhaps you are trying to encode the non-representation version of a node?"

}
tk.Type = tok.TNull
_, err := sink.Step(&tk)
return err
Expand Down
5 changes: 4 additions & 1 deletion codec/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ func Marshal(n ipld.Node, sink shared.TokenSink) error {
func marshal(n ipld.Node, tk *tok.Token, sink shared.TokenSink) error {
switch n.Kind() {
case ipld.Kind_Invalid:
return fmt.Errorf("cannot traverse a node that is absent")
return fmt.Errorf("invalid node encountered")
case ipld.Kind_Null:
if n.IsAbsent() {
return fmt.Errorf("cannot encode a node that is absent")
}
tk.Type = tok.TNull
_, err := sink.Step(tk)
return err
Expand Down
2 changes: 1 addition & 1 deletion unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var Absent Node = absentNode{}
type absentNode struct{}

func (absentNode) Kind() Kind {
return Kind_Null
return Kind_Null // It's questionable if this is a good idea. Perhaps it should be Kind_Invalid. See https://github.com/ipld/go-ipld-prime/issues/191 .
}
func (absentNode) LookupByString(key string) (Node, error) {
return nil, ErrWrongKind{TypeName: "absent", MethodName: "LookupByString", AppropriateKind: KindSet_JustMap, ActualKind: Kind_Null}
Expand Down