should push_back/pushKV methods throw for wrong object types?

[ryanofsky]

I was surprised to see it's not an error to call push_back on UniValues that aren't arrays and pushKV on UniValues that aren't objects. Is this by design? It caused a bug recently: bitcoin/bitcoin#10450

[jgarzik]

Yes, that's largely by design.

1. Exceptions are expensive, and
2. Typically UniValues do not change types at runtime, implying this pattern is a programmer error. That implies an assert() might be a better choice than an exception.

Other opinions and comments are solicited.

[ryanofsky]

I didn't realize push methods already do have a return value indicating success. So failure is at least detectable, even if there is some reasoning for treating it like a programmer error instead of a runtime error.

Since it doesn't appear there are any asserts used so far in univalue code, and asserts aren't easy to catch in unit testing, and since there would be some overhead in throwing a std::logic_error, I think I'd be inclined not to make any changes, and will close this issue. Thanks for taking a look.