[core-tracing] Fix the return type of withSpan

[maorleger]

Packages impacted by this PR

@azure/core-tracing

Issues associated with this PR

N/A - created after discussion with some team members

Describe the problem that is addressed by this PR

withSpan is meant to await some user-provided callback and set the appropriate
status on the created span before closing it. Because we have no way of knowing
whether the callback is sync or async we must promisify it first. Up until now that
meant that the return type of a promise-returning callback was
Promise<Promise<...>> which does not reflect the reality that await will
recursively unwrap promises. That meant that the consumer must also either
await or mark the function as async in order to avoid a return-type
mismatch.

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The solution here is to introduce an AwaitedLike type which is a narrower
version of TypeScript 4.5's Awaited type. Because our min-bar is lower we
cannot take advantage of this new type yet, so I introduced something like it
which can be replaced with the broader type once we support TS >= 4.5.

Are there test cases added in this PR? (If not, why?)

We already have test cases for this function for both sync and async callbacks,
as this is a pure type-change with no runtime behavior change no test cases are
needed

Checklists

• Added impacted package name to the issue description
• Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
• Added a changelog (if necessary)

[maorleger]

This is not needed in TS 4.4 see playground link but for reasons unknown to me TS4.2 infers the returntype of this as unknown

[joheredi]

Thanks for sharing the playground!

[azure-sdk]

API changes have been detected in @azure/core-tracing. You can review API changes here

API changes

+ export declare type AwaitedLike<T> = T extends object & {
+     then(onfulfilled: infer F): any;
+ } ? F extends (value: infer V) => any ? AwaitedLike<V> : never : T;
-         }, Callback extends (updatedOptions: Options, span: Omit<TracingSpan, "end">) => ReturnType<Callback>>(name: string, operationOptions: Options, callback: Callback, spanOptions?: TracingSpanOptions): Promise<ReturnType<Callback>>;
+         }, Callback extends (updatedOptions: Options, span: Omit<TracingSpan, "end">) => ReturnType<Callback>>(name: string, operationOptions: Options, callback: Callback, spanOptions?: TracingSpanOptions): Promise<AwaitedLike<ReturnType<Callback>>>;

[joheredi]

These comments are great! Thanks for documenting this type 😄

[joheredi]

Looks good! Do we expect this to be a breaking change?

[maorleger]

Looks good! Do we expect this to be a breaking change?

In this case we're OK because these are all new and unreleased APIs 👍

[witemple-msft]

LGTM but with a couple of notes about how the type is defined.

[witemple-msft]

AwaitedLike is somewhat strange as a name since it makes it sound like an interface for something that is "like an Awaited instance." Maybe something like UnwrapThenable if you really don't want to overlap with TS4.5's Awaited name.

Since tracing is in beta we can remove this in the future if we decide that we can rely on TS4.5's definition.

[witemple-msft]

Also, since { then: ... } is a subtype of object, there shouldn't be any difference between object & { then: ... } and { then: ... }.

[maorleger]

Also, since { then: ... } is a subtype of object, there shouldn't be any difference between object & { then: ... } and { then: ... }.

Thanks, yeah good call!

As far as the name... maybe AwaitedThenable? I like having Awaited in the name what do you think?

[maorleger]

I was chatting with Jeff and we went with Resolved<T> which feels appropriate here where the returntype is:

Promise<Resolved<ReturnType<Callback>>>;

[azure-sdk]

API changes have been detected in @azure/core-tracing. You can review API changes here

API changes

+ export declare type AwaitedLike<T> = T extends {
+     then(onfulfilled: infer F): any;
+ } ? F extends (value: infer V) => any ? AwaitedLike<V> : never : T;
-         }, Callback extends (updatedOptions: Options, span: Omit<TracingSpan, "end">) => ReturnType<Callback>>(name: string, operationOptions: Options, callback: Callback, spanOptions?: TracingSpanOptions): Promise<ReturnType<Callback>>;
+         }, Callback extends (updatedOptions: Options, span: Omit<TracingSpan, "end">) => ReturnType<Callback>>(name: string, operationOptions: Options, callback: Callback, spanOptions?: TracingSpanOptions): Promise<AwaitedLike<ReturnType<Callback>>>;

[azure-sdk]

API changes have been detected in @azure/core-tracing. You can review API changes here

API changes

+ export declare type Resolved<T> = T extends {
+     then(onfulfilled: infer F): any;
+ } ? F extends (value: infer V) => any ? Resolved<V> : never : T;
-         }, Callback extends (updatedOptions: Options, span: Omit<TracingSpan, "end">) => ReturnType<Callback>>(name: string, operationOptions: Options, callback: Callback, spanOptions?: TracingSpanOptions): Promise<ReturnType<Callback>>;
+         }, Callback extends (updatedOptions: Options, span: Omit<TracingSpan, "end">) => ReturnType<Callback>>(name: string, operationOptions: Options, callback: Callback, spanOptions?: TracingSpanOptions): Promise<Resolved<ReturnType<Callback>>>;