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
2.02k stars 1.19k forks source link

[swagger]containerApps.listSecrets throws response code 204 as an error #21101

Closed nturinski closed 2 years ago

nturinski commented 2 years ago

Describe the bug client.containerApps.listSecrets considers the 204 response code to be an error. This happens if the container app pinged has no secrets associated with it. It should probably just return an empty array rather than throwing an error.

To Reproduce Steps to reproduce the behavior:

  1. Call client.containerApps.listSecrets on a container app that has no secrets

Expected behavior If the container app has a secret, return the array. If it does not, return an empty array.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

azure-sdk commented 2 years ago

Label prediction was below confidence level 0.6 for Model:ServiceLabels: 'App Services:0.43074873,Storage:0.09991559,Container Instances:0.09626426'

nturinski commented 2 years ago

Note: This also occurs with containerApps.beginDeleteAndWait which would also return 204 once the content is deleted.

qiaozha commented 2 years ago

Hi @MaryGao Can you take a look at this issue ? Thanks

MaryGao commented 2 years ago

@nturinski Thanks for reporting this! I am investigating this and it seems this is the service by-design behavior.

nturinski commented 2 years ago

The 204 seems by-design, but should it return as an error? Right now in order to use this API endpoint, I have to wrap it in a try/catch, swallow the error, and return an empty array.

MaryGao commented 2 years ago

@nturinski I tried in my local, Are these behaviors expected from your side?

nturinski commented 2 years ago

For the operation client.containerApps.listSecrets, it will throw the RestError with 204 header

This is unexpected behavior. 204 means it is an empty (no content) response and shouldn't be considered an error. You will get this response if your container app doesn't contain any secrets.

For the operation containerApps.beginDeleteAndWait, it will return undefined and NOT throw any error with 204 header

This is expected behavior and is great!

MaryGao commented 2 years ago

@nturinski Hi Nathan, thanks for reporting this and the first one is identified as a swagger issue and the swagger definition didn't take 204 as success status code into consideration and we contacted the service team and they would fix the swagger then we'll release a new version.

MaryGao commented 2 years ago

As per the original issue, this also applies to some delete methods in the arm-appcontainers SDK. That includes:

managedEnvironments.beginDeleteAndWait containerApps.beginDeleteAndWait

@nturinski Could you create an issue for the package arm-appcontainers considering they are different packages so we could better track their status?

nturinski commented 2 years ago

I believe it's the same package, but it was renamed. Could I edit my first comment to reflect the change in package name and version? Everything else is still valid.

MaryGao commented 2 years ago

@nturinski Yes, you are right and this is re-named package. Please edit your first comment to reflect them. Thanks!

MaryGao commented 2 years ago

As per the original issue, this also applies to some delete methods in the arm-appcontainers SDK. That includes:

managedEnvironments.beginDeleteAndWait containerApps.beginDeleteAndWait

@nturinski Could you suggest why you add the two deletion operations again?

After checking the original issue, I think we have an agreement that there is no change for containerApps.beginDeleteAndWait and the below description is expected behavior:

I'd like to highlight that this would work for operation managedEnvironments.beginDeleteAndWait also.

nturinski commented 2 years ago

@nturinski Could you suggest why you add the two deletion operations again?

I think I misunderstood the first time. Currently, beginDeleteAndWait doesn't return undefined and does throw an error.

image

'RestError: Received unexpected HTTP status code 204 while polling. This may indicate a server issue.\n\tat isUnexpectedPollingResponse (c:\\Users\\naturins\\Code\\vscode-azurecontainerapps\\node_modules\\@azure\\core-lro\\dist\\index.js:464:15)\n\tat isLocationPollingDone (c:\\Users\\naturins\\Code\\vscode-azurecontainerapps\\node_modules\\@azure\\core-lro\\dist\\index.js:549:13)\n\tat processLocationPollingOperationResult (c:\\Users\\naturins\\Code\\vscode-azurecontainerapps\\node_modules\\@azure\\core-lro\\dist\\index.js:552:63)\n\tat GenericPollOperation.poll (c:\\Users\\naturins\\Code\\vscode-azurecontainerapps\\node_modules\\@azure\\core-lro\\dist\\index.js:595:16)\n\tat processTicksAndRejections (node:internal/process/task_queues:96:5)\n\tat async GenericPollOperation.update (c:\\Users\\naturins\\Code\\vscode-azurecontainerapps\\node_modules\\@azure\\core-lro\\dist\\index.js:683:34)\n\tat async LroEngine.pollOnce (c:\\Users\\naturins\\Code\\vscode-azurecontainerapps\\node_modules\\@azure\\core-lro\\dist\\index.js:200:34)'

For the operation containerApps.beginDeleteAndWait, it will return undefined and NOT throw any error with 204 header

I think that this should be the expected behavior.

MaryGao commented 2 years ago

@nturinski we reproduced the errors in local and these two deletions are identified as client errors. We are working on the fixes and will let you know once ready!

Please notice this error is different from the one in client.containerApps.listSecrets which is a swagger issue we need to work with service team to fix.

MaryGao commented 2 years ago

@nturinski Could you try to install the latest 1.1.0 version and update its dependency to latest?

We fixed the DELETE issue in client side what's more we found that client.containerApps.listSecrets is fixed from server side.

Feel free to have a try and let us know if it works for you!

nturinski commented 2 years ago

Verified that this is fixed. Thanks!

qiaozha commented 2 years ago

close it as issue addressed. Thanks