actions / http-client

A lightweight HTTP client optimized for use with actions, TypeScript with generics and async await.
https://github.com/features/actions
Other
72 stars 33 forks source link

Authentication handlers should match Typescript interface #21

Open singingwolfboy opened 4 years ago

singingwolfboy commented 4 years ago

The authentication handlers in auth.ts all define handleAuthentication methods that look like this:

handleAuthentication(httpClient: ifm.IHttpClient, requestInfo: ifm.IRequestInfo, objs): Promise<ifm.IHttpClientResponse> {
    return null;
}

However, this is not strictly valid Typescript, since the actual return type does not match the declared return type. Notice that the actual return type is null, while the declared return type is Promise<ifm.IHttpClientResponse>.

Fixing this issue may require modifying the Typescript types to better describe intent. Here's one way we could do it:

export interface IRequestHandlerWithAuth {
  prepareRequest(options: http.RequestOptions): void;
  canHandleAuthentication(response: IHttpClientResponse): true;
  handleAuthentication(
    httpClient: IHttpClient,
    requestInfo: IRequestInfo,
    objs: any
  ): Promise<IHttpClientResponse>;
}

export interface IRequestHandlerWithoutAuth {
  prepareRequest(options: http.RequestOptions): void;
  canHandleAuthentication(response: IHttpClientResponse): false;
}

export type IRequestHandler =
  | IRequestHandlerWithAuth
  | IRequestHandlerWithoutAuth;

Notice how there are now two different kinds of authentication handlers: with and without auth. If a handler does not support authentication, it does not need to implement the handleAuthentication() method!