Incorrect return type definition for `deleteKey`

[CarstenLeue]

The type definition for deleteKey is:

    deleteKey(params: IbmKeyProtectApiV2.DeleteKeyParams): Promise<IbmKeyProtectApiV2.Response<IbmKeyProtectApiV2.DeleteKey>>;

with IbmKeyProtectApiV2.DeleteKey:

    interface DeleteKey {
        /** The metadata that describes the resource array. */
        metadata: CollectionMetadata;
        /** A collection of resources. */
        resources: KeyWithPayload[];
    }

but the actually returned data is:

{
    "status": 204,
    "statusText": "No Content",
    "headers": {
        "access-control-allow-headers": "Origin, Content-Type, Bluemix-Space, Bluemix-Org, Authorization, Prefer, Request-Origin",
        "access-control-allow-methods": "GET, POST, DELETE, OPTIONS, HEAD",
        "access-control-allow-origin": "*",
        "access-control-expose-headers": "Correlation-Id, Key-Total",
        "bluemix-instance": "14f860fb-ef1c-4814-be09-58c6d269389a",
        "cache-control": "no-cache, no-store, must-revalidate",
        "correlation-id": "931bbe03-eb0b-4d5a-9374-23fd8200c213, 931bbe03-eb0b-4d5a-9374-23fd8200c213",
        "date": "Mon, 29 Nov 2021 16:05:32 GMT",
        "expires": "0",
        "pragma": "no-cache",
        "strict-transport-security": "max-age=31536000; includeSubDomains; preload",
        "connection": "close"
    },
    "result": ""
}

Clearly result is not of the specified data type.

[padamstx]

This scenario is an unfortunate consequence of the API definition containing multiple success responses for the deleteKey operatoin... a 200 which returns a response object, and a 204 with no response object. Because the SDK generator has to generate one and only one method for the operation it uses only the primary success response (the 200 entry with the response object). In this case, you received a 204 response and the "result" field within that raw response object is not present due to the fact that the server doesn't send a response object for that status code.

[CarstenLeue]

Hi @padamstx while I understand the technical reason for this, I think that this aspect still should be addressed, because imho we should make sure that the type definition of the API matches the real data structures. Otherwise how would I code against the API? I would have to code with the premise in mind that there exists a mismatch between the API and the data. In that case it would be better to declare the response as any.

From your explanation the root cause for this issue is a shortcoming of the API generator. I have not looked into every detail, but based on the description a better type definition would be:

    deleteKey(params: IbmKeyProtectApiV2.DeleteKeyParams): Promise<IbmKeyProtectApiV2.Response<IbmKeyProtectApiV2.DeleteKey | string>>;

this is still not perfect, because it does not define the circumstances under which you'd expect a string vs a data structure, but it would at least give a hint to expect a string.

[padamstx]

In this situation, the service team really can't update the API definition, because it currently reflects the behavior of the service (i.e. the operation can return a 200 status code along with an instance of the DeleteKey schema in the reponse body, or a 204 status code with no response body at all). Unfortunately, the SDK generator uses only the first success response when generating client-side code for an API, and while we theoretically could use a type union to define the return type of the method in node.js, we can't do that in most of the other languages because it just isn't supported (i.e. Go, Java, etc.). There are several examples of this type of situation with other IBM Cloud services and unfortunately, it's not something we can fix in the SDK generator.

[CarstenLeue]

Unfortunately, the SDK generator uses only the first success response when generating client-side code for an API

I do not want to ride this to death, but why can't we fix the SDK generator? If there exist languages that do not support type unions, then the correct type for result is the equivalent of the unknown type for that language. If a language supports unions, then that's the right way to go. I do not really follow the argument that because of a shortcoming of the SDK generator we are generating an incorrect type. From my perspective no type information is better than an incorrect one, because the incorrect one lures you into false certainty. In effect I have to cast this result to any and then do a runtime type check, so the current type is more harm than benefit.

[padamstx]

I do not want to ride this to death, but why can't we fix the SDK generator?

The primary reason is that our SDK generator is built on the openapi-generator open source code, and this behavior comes from that underlying code. I realize that's still not a "good" reason. Perhaps in the future we can look at modifying this behavior, but it would require a fairly large effort due to the need to address first in the underlying openapi-generator code. I'm sorry that we don't generate perfect code in each language, but we do the best that we can given all the constraints that we operate under.

In effect I have to cast this result to any and then do a runtime type check, so the current type is more harm than benefit.

I think it's even easier than that... i.e. if you receive a 200 status code (you can check this by looking at the "status" field of the response object returned to you), then you should be able to assume that the response object's "result" field contains a non-null instance of the return type (DeleteKey); OTOH if you receive a 204 status code, then you can assume there's no "result" value returned in the response. This is essentially how the operation is defined in the API definition, although it is not clearly implemented that way in the generated SDK code.