Re generate cadl ranch tests #8
Conversation
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
| if (isUnexpected(response)) { | ||
| throw createRestError( | ||
| `Pagination failed with unexpected statusCode ${response.status}`, | ||
| response, | ||
| ); | ||
| const statusCode = Number(response.status); | ||
| if (statusCode < 200 || statusCode >= 300) { | ||
| if (statusCode >= 400 && statusCode < 500) { | ||
| throw createRestError( | ||
| `Pagination failed with client error statusCode ${response.status}`, | ||
| response, | ||
| ); | ||
| } else if (statusCode >= 500) { | ||
| throw createRestError( | ||
| `Pagination failed with server error statusCode ${response.status}`, | ||
| response, | ||
| ); | ||
| } else { | ||
| throw createRestError( | ||
| `Pagination failed with unexpected statusCode ${response.status}`, | ||
| response, | ||
| ); | ||
| } |
There was a problem hiding this comment.
not quite sure why we remove isUnexpected helper ?
There was a problem hiding this comment.
is it part of the removing rlc types change?
| @@ -1,20 +1,105 @@ | |||
| // Copyright (c) Microsoft Corporation. | |||
| // Licensed under the MIT license. | |||
There was a problem hiding this comment.
If I remember correctly, not everything is static in paging helpers, for example
There was a problem hiding this comment.
this nextlinkname option should be fixed in paging helper but could be configed differently in places called this helper -
| PagedAsyncIterableIterator, | ||
| } from "./models/index.js"; | ||
| export { User, ListOptionalParams } from "./models/index.js"; | ||
| export { PageSettings, ContinuablePage, PagedAsyncIterableIterator }; |
There was a problem hiding this comment.
Just curious, why we import first and then export them instead of export them directly?
| const Http2xxStatusCodes = [ | ||
| "200", | ||
| "201", | ||
| "202", | ||
| "203", | ||
| "204", | ||
| "205", | ||
| "206", | ||
| "207", | ||
| "208", | ||
| "226", | ||
| ]; |
There was a problem hiding this comment.
I wonder if that means we have supported range status Azure#2063 @MaryGao ?
There was a problem hiding this comment.
The issue you linked is not the same thing as this range, 2063 one is talking about supporting status range from typespec. But this helper is deviring from the logic in HLC: https://github.com/Azure/autorest.typescript/blob/main/packages/autorest.typescript/test/smoke/generated/purview-administration-rest/src/account/paginateHelper.ts#L135
There was a problem hiding this comment.
yeah, I wonder if that could cause some problem since we are expanding the valid response status code for 2xx?
There was a problem hiding this comment.
I would say that could be a pontential issue but I am not worried about this. The above logic is used in HLC for a long time to check if the response is okay or not, and it works well.
Splitting this into its own PR to help reviewing