Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
1.99k stars 1.16k forks source link

[@azure/arm-desktopvirtualization] Nullable Properties #27068

Open elliot-huffman opened 10 months ago

elliot-huffman commented 10 months ago

Describe the bug The documentation and typings are incorrect for these objects: Host Pool: https://learn.microsoft.com/en-us/rest/api/desktopvirtualization/host-pools/get?tabs=HTTP#hostpool

Session Host: https://learn.microsoft.com/en-us/rest/api/desktopvirtualization/session-hosts/get?tabs=HTTP#sessionhost

The properties are nullable but are not documented as such. Instead, the API docs and typings says they are optional properties, which is different from null. in JavaScript/TypeScript undefined !== null As a workaround, I must override the schema of the resultant object so that TypeScript doesn't freak out when I compare against null instead of Undefined.

To Reproduce Steps to reproduce the behavior:

  1. Run a web request with an un-assigned user on a session host resource or registration key on a host pool resource.
  2. They return null which is undocumented and not in the typings.
  3. Other properties return null too that are also undocumented, but I really don't care about the quality of the docs for those since they don't affect me. (you can see some other properties return null in the screenshot, I would highly recommend reviewing all properties and their return values when triaging this issue to find many more issues with the docs/typings that I just don't care enough to report.)

Expected behavior The Documentation and typings reflect the actual state of the API.

Screenshots Screenshot of the Host Pool's registration info property returning null when no valid registration key is set. Other null values are noted for your reference as this is a rampant issue that affects more than just my stuff: Screenshot of various null values being returned, contrary to the docs

Screenshot of a session host object with null values present: Screenshot of various null values being returned, contrary to the docs

Additional context This should not be a server-side fix, just a client and docs fix. May require updating the OpenAPI spec that generates the clients. Also, the skip token is not documented properly, even though it exists, just a separate thing to note.

Below is some workaround code to hot patch this behavior, this should not be needed if the typings were correct.

Host Pool Workaround:

/** Corrected type information for the registration info object with expiration time property with the right data. */
type BetterRegistrationInfo = {
    'expirationTime'?: Date | null;
} & Omit<RegistrationInfo, 'expirationTime'>;

/** Host Pool object but with proper type for the registration info property. */
export type BetterHostPool = {
    'registrationInfo': BetterRegistrationInfo;
} & Omit<HostPool, 'registrationInfo'>;

Session Host Workaround:

/** Session Host object but with the proper type for the assigned user property. */
export type BetterSessionHost = {
    'assignedUser'?: string | null;
} & Omit<SessionHost, 'assignedUser'>;
xirzec commented 10 months ago

@qiaozha Looks like another inaccurate service description

qiaozha commented 10 months ago

@xirzec I think you are right and as they are retiring S360 which is used to make sure service backend behaves the same as their rest api specs, I am afraid we will get more and more issues like this. 😭

github-actions[bot] commented 10 months ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @armleads-azure.

qiaozha commented 10 months ago

potential work for x-ms-null in codegen. @kazrael2119 Could you create an issue in autorest typescript repo ?