Azure / autorest.typescript

Extension for AutoRest (https://github.com/Azure/autorest) that generates TypeScript code. The transpiled javascript code is isomorphic. It can be run in browser and in node.js environment.
MIT License
176 stars 75 forks source link

`isUnexpected` should handle the streaming responses #2008

Open deyaaeldeen opened 1 year ago

deyaaeldeen commented 1 year ago

isUnexpected is generated with overloads where each one corresponds to a REST API call. However, it doesn't take into account situations when the customer calls .asNodeStream or .asBrowserStream. We expect overloads for HttpNodeStreamResponse and HttpBrowserStreamResponse to be supported.

Here is my proposal:

export interface DefaultResponse extends HttpResponse {
  status: string;
  body: ErrorResponse;
  headers: RawHttpHeaders & DefaultHeaders;
}

export function isUnexpected(
  response: HttpBrowserStreamResponse | HttpNodeStreamResponse | DefaultResponse
): response is DefaultResponse;
MaryGao commented 11 months ago

Thanks for the proposal! The current implementation didn't consider the streaming situations becuase every operation could return HttpNodeStreamResponse and HttpBrowserStreamResponse. So the type systems may fail to suggest the unexpected response if different operations have different default response shapes. The type system would always return the first one matched.

export function isUnexpected(
  response:
    | StringGetNotProvided200Response
    | StringGetNotProvidedDefaultResponse
    | HttpBrowserStreamResponse // Adding stream response
    | HttpNodeStreamResponse
): response is StringGetNotProvidedDefaultResponse;
export function isUnexpected(
  response:
    | StringGetBase64Encoded200Response
    | StringGetBase64EncodedDefaultResponse
    | HttpBrowserStreamResponse // Adding stream response
    | HttpNodeStreamResponse
): response is StringGetBase64EncodedDefaultResponse;

I may re-think if we have a better way to solve this and feel free to share your ideas!