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.17k forks source link

[AppConfig] listing configuration uses wrong page etag. #30282

Closed Eskibear closed 1 week ago

Eskibear commented 3 weeks ago

Describe the bug To list configuration settings with page etags, requests are always sent with etag of the first page.

To Reproduce Steps to reproduce the behavior:

  1. add 100+ configuration settings, then we have more than 1 page.
  2. list configuration settings by page, to get the page etags.
  3. list configuration settings by page again, providing the pageEtags option.
  4. print status code of each page, you'll see it's 200 for the 2nd page.

Expected behavior all 304, because nothing changes.

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

Additional context

Culprit: https://github.com/Azure/azure-sdk-for-js/blob/f58e6bf6849b38ace148d9ea53c4b137a2dad235/sdk/appconfiguration/app-configuration/src/appConfigurationClient.ts#L337-L346

The parameter etag is unchanged for each request. You should call etag = pageEtags.shift() for each page.

minhanh-phan commented 3 weeks ago

Hi @Eskibear, thank you for raising the issue. I'll try to reproduce it and get back to you. Thank you for your patience!

minhanh-phan commented 2 weeks ago

Hi @Eskibear, thank you for raising the issue. We have released version 1.6.1 patch for this bug. Please give it a try and let us know if you are still seeing this error. Thank you for your patience.

github-actions[bot] commented 2 weeks ago

Hi @Eskibear. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

Eskibear commented 1 week ago

thanks for the quick fix. verified it's working as expected now.

minhanh-phan commented 1 week ago

I'm glad that it is working as expected. I'll close this issue as done!