FixedWidth: check for fields during serialization

[rf]

Noticed that when attempting to serialize a struct with a FixedWidth and that FixedWidth wasn't specified in the object we'd get a garbage exception. This adds in a check to make sure fields that are named are actually specified in the object, so we don't try to get the byte length of something undefined, nor do we attempt to serialize something that's undefined.

[jcorbin]

Nice catch.

While I totally agree that bufrw should be hardened against this case, note that I consider designing your struct class such that it can have such an undefined hole to itself be a poor choice for that class.

In TChannel, every class that uses struct initializes every field to some sensible "zero" initial value in the constructor. This helps with performance as it carves out a clear order-of-definition for v8 to sort out the class shape and type hinting. Furthermore it would make this case harder to get into as someone would have to expressly nullify or assign undefined the field before calling .byteLength / .writeInto. E.g. https://github.com/uber/tchannel/blob/bufrw/node/v2/call.js#L47

[rf]

Of course it's important and necessary for the class to guard against these errors, but in this case, the class was accidentally passing in a value of the wrong type as the default case; so in the case of programmer error (that will most likely have occurred during development) we get an undecipherable exception, which is precisely what happened here. reqres.js was initialized with a Buffer for tracing (emptyTracing) rather than an object, and so the undecipherable error surfaced when I ran the tests. So this is absolutely necessary to be in bufrw and it should guard against all such cases.

Comments addressed

[jcorbin]

:+1: I refactored the test one last time to use the normal bufrw error checking paths.

[jcorbin]

Published in 0.9.5