-
Notifications
You must be signed in to change notification settings - Fork 650
Description
Describe the feature
AWS services that mark their response members @required are forced to keep that field indefinitely, for backwards compatibility reasons, as noted in this comment: #6238 (comment). However, the current workaround is not how modern SDKs work, and it is a bad experience.
Casting is "lying" a bit, and any amount of casting in the client-code is deluding everything using that object about its shape. The current workaround suggested in #6238 (comment) is to cast the client to AssertiveClient. I think this class is a misnomer, it's not asserting anything! It's just fibbing to the client about the types that are really returned.
If the on-the-wire format and deserialization logic in JavaScript enforces that a certain field be present, then the TypeScript types should reflect that, with member: T instead of member: T | undefined, with no casting. That is, deserialization without validation leads to bad experiences. The deserialization result should match the type exactly, similar to TypeScript's Zod or Rusts' Serde philosophy.
Casting types at the edges of a system (network calls) means that developers often can't really trust the return value, they just hope the members they need really exist.
Use Case
Client libraries are meant to offload common operations to interact with a service into the library and away from everyday users. I think deserialization + parsing is in that camp. All users care about the required members of the services they use, and requiring the same:
import { ok } from "node:assert";
const res = s3.getObject(...);
ok(res.Body);
... do something with res.Body ...every single time is tiresome!
Rust's SDK does better here, and all values returned "just work", since the service has promised to return them. That's what @required means!
Proposed Solution
I would love a ParsingClient or a flag in each generated client that I could put to ensure that:
- The return type has all
@requiredmembers asmember: T, and - The JavaScript actually enforces that and will throw parsing exceptions if the return type is anything other than
{ member: T }.
This is how modern SDKs like Rust work, and JavaScript not working this way is a worse developer experience for all that use the SDK.
I may have misunderstood the deserialization logic. If my second ask (parsing) is already the case, then why not update the types for all clients, not just AssertiveClient?
Other Information
I admit that it's a bit difficult to handle a case like:
input PutThingRequest {
thing: Thing
}
output PutThingResponse {
thing: Thing
}
structure Thing {
@required // How to have it required only on input? Or only on output?
member: String
}which is common with "boilerplate" types like common collections or small compositions. If that's the blocker, I would ask the Smithy team to introduce @requiredInput and @requiredOutput, update AWS SDK models, and show that to the client. That would mean that Smithy is not modelling the world as well as it could be.
Acknowledgements
- I may be able to implement this feature request
- This feature might incur a breaking change
SDK version used
3.883.0
Environment details (OS name and version, etc.)
NodeJS, x86 and aarch64