New transaction `getMulti.exists()`should not throw

[JesusTheHun]

The whole purpose of getMulti.exists() is to check the existence of an element, so it should not throw if the element does not exist, out of bounds or not. This is especially annoying in the case of a dynamic number of documents.

The contentAt method is legit to throw, even though returning undefined is a perfectly valid option (because it's an illegal json value) but there is the case for binary content that could be decoded as undefined

[thejcfactor]

The code is following the transactional GetMulti specification. So, I don't know if there will be a change. However, can you provide details on the use-case for providing an index that is out of bounds? In order to use the transactional GetMulti operation the application must provide a list of specs, so the application knows appropriate bounds. Why would the application need to check for existence outside of those bounds?

[JesusTheHun]

It's just for convenience. Interacting with the same interface is better than juggling between the spec array and the result object.

I noticed something else: exists checks for the value to be undefined, but custom transcoder could deserialize a document to undefined. So what should be checked is that error is undefined rather than checking that value is not undefined.

Also, having errorAt seems natural, since you want to abstract away the array of results

[thejcfactor]

but custom transcoder could deserialize a document to undefined. So what should be checked is that error is undefined rather than checking that value is not undefined.

Fair point. Created JSCBC-1351 to make the appropriate changes. Will be available in the 4.5.0 release.

Would .exists(strict=false) work for convenience? This allows for the default to align w/ the specification and also provide a mechanism for users to have exists() behave more "loosely" if desired. If this works, we should be able to get this in on the first patch of the 4.5 branch. I need to confer w/ others on the team as well.

[JesusTheHun]

A flag could work.

The wording here is really not ideal.
Throwing for contentAt is fine because you don't have to deal with a type guard with TypeScript, but exists should really be named hasContent to avoid the ambiguity of "does the index exists or the content itself ?". THEN it would make sense to throw for hasContent when the index does not exists.

I didn't find the RFC by the way.

[thejcfactor]

The changes associated with JSCBC-1351 are now available with v4.5.0 of the Node.js SDK.

Unfortunately, I don't think the transactions RFC is publicly available.

I will keep this issue open until I have had a chance to have a better discussion about the Node.js client deviating from the spec.