`types.Float64Value(math.NaN())` causes a `panic`: expected behavior or bug?

[mschuchard]

Currently the Float64Value() constructor in the types package panics when the input is math.NaN(). math.NaN() does return a float64, but this is also kind of an edge case to be sure. What is interesting to me though is that types.Float64Value(math.Inf(1)) behaves as expected, and is a similar edge case.

Is this expected behavior or a bug? I encountered this during unit testing an edge case to ensure it errors appropriately in case there is any question why and/or how this occurred.

[austinvalle]

Hey there @mschuchard 👋🏻 ,

I'd say this is expected and more of a rough edge for our Float64Value() function, as it doesn't return diagnostics (like say ObjectValue()), so we can't safely check/return an error here like we'd probably prefer to in this exact edge case (for example):

func NewFloat64Value(value float64) (Float64Value, diag.Diagnostics) {
	if math.IsNaN(value) {
		return NewFloat64Unknown(), diag.Diagnostics{
			diag.NewErrorDiagnostic("Invalid float64 value", "cannot encode NaN"),
		}
	}
	return Float64Value{
		state: attr.ValueStateKnown,
		value: big.NewFloat(value),
	}
}

Framework encodes Float64Value to a Terraform Number over the protocol using a *big.Float, which we don't currently support encoding NaN as an edge case. On the other side, Terraform core uses go-cty, which also doesn't support NaN.

I believe it could be possible to support NaN in general, if there was a strong use-case, because MsgPack supports it (we'd need to update both core and the provider type systems):

• https://github.com/msgpack/msgpack/blob/master/spec.md#type-system

---

As for your note on positive infinity, that is something we support and can encode, as both terraform-plugin-go (provider type system) and go-cty (HCL/Terraform type system) support that via msgpack:

• https://github.com/hashicorp/terraform-plugin-go/blob/6ebe79b1c819d7712d0eb05729e962575687c589/tftypes/value_msgpack.go#L430-L443
• https://github.com/zclconf/go-cty/blob/67d85b647e726fed180cfeeb63161cb341582f36/cty/msgpack/marshal.go#L84-L87

[austinvalle]

I'm not sure if you were looking explicitly for NaN support, so I'm going to close the issue. If that's what you were looking for, we can re-open and then we'd need to start a new issue on the Terraform core repo to discuss the use-cases.

If your goal was to protect your provider function code, then you should be good as Terraform core can't encode a NaN to send to the provider 👍🏻

[mschuchard]

I'm not sure if you were looking explicitly for NaN support, so I'm going to close the issue.

This was a bug(?) report and not a feature request, and so the response that this is expected behavior is 100% satisfactory. I would have initially expected this to be expected behavior except for the proper handling of infinity (similar situation in my opinion), and so I opened the issue in case I had found something unexpected and unknown from current code and testing coverage.

The references to go-cty support are also super relevant and convincing since I am super aware from my Packer plugin work how dependent the Hashi stack is on that module for arbitrary HCL2 encoding and decoding.

Thanks; we are definitely good to go here.