[Service Bus] Generic types for Responses for a cleaner API surface

[HarshaNalluru]

[richardpark-msft]

No opinion on this yet but I think this hinges on what you decide as far as how we want to handle Response.

1. We can inline Response's fields which will remove one interface from the top level surface.
2. We can use this generics method. Curious what the docs look like for it.
3. We can just do the extends methodology.

[ramya-rao-a]

cc @bterlson, @xirzec

[HarshaNalluru]

We can use this generics method. Curious what the docs look like for it.

Example Method

Response Type in the Docs

@richardpark-msft

[xirzec]

I love using a generic for this, as I think it looks a lot better.

My only suggestion is maybe the response interface is too broad for what we want to expose, though I recognize that you haven't changed it from the previous release. Maybe we can discuss how to standardize this across libraries tomorrow?

[xirzec]

shouldn't this be HttpResponse instead? HttpOperationResponse has a lot of internal details that don't add much value to the consumer, whereas HttpResponse is enough to get headers, status, and the original request.

[HarshaNalluru]

Hmmm.. we never cared enough to update. We can make it HTTPResponse.

For reference,

// Service Bus
_response: HttpOperationResponse;

// From core-http
export interface HttpOperationResponse extends HttpResponse {
  parsedHeaders?: { [key: string]: any };
  bodyAsText?: string | null;
  parsedBody?: any;
  blobBody?: Promise<Blob>;
  readableStreamBody?: NodeJS.ReadableStream;
}

// Example for `_response` in storage 
_response: HttpResponse & {
  parsedHeaders: ServiceGetUserDelegationKeyHeaders;
  bodyAsText: string;
  parsedBody: UserDelegationKeyModel;
};

Maybe we should consider standardizing the _responses?

[HarshaNalluru]

Since this is beyond the scope of this PR, logging a new issue to update to _response: HTTPResponse - #10841
I'll put up a PR for this if nobody has objections.

[HarshaNalluru]

Response from the proposal has been renamed to WithResponse.

[HarshaNalluru]

Feedback from @bterlson (from offline):

• WithResponse is fine, _response: HTTPResponse (as @xirzec suggested) is also fine
• Suggested a generic constraint on WithResponse<T>
  • Example: WithResponse<number> should be an error
  • Can do T extends object

[deyaaeldeen]

One thing to consider here is the inconsistency in how to provide _response to our users as tracked by this issue: #10887. This is already an existing issue so I think there is no reason to block this PR as is.

[HarshaNalluru]

Thanks, @deyaaeldeen.
I will push updating _response: HTTPResponse to a follow up PR.

[ghost]

Hello @HarshaNalluru!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[HarshaNalluru]

/check-enforcer override