Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clearer middleware chain #1729

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/CustomMiddlewareChain.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { Middleware } from "@microsoft/microsoft-graph-client";
import { Context } from "@microsoft/microsoft-graph-client";

export class MyLoggingHandler implements Middleware {
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

public async execute(context: Context): Promise<void> {
try {
Expand All @@ -29,7 +29,7 @@ export class MyLoggingHandler implements Middleware {
url = context.request.url;
}
console.log(url);
return await this.nextMiddleware.execute(context);
return await this.nextMiddleware?.execute(context);
} catch (error) {
throw error;
}
Expand Down Expand Up @@ -117,7 +117,7 @@ import { Middleware } from "@microsoft/microsoft-graph-client";
import { Context } from "@microsoft/microsoft-graph-client";

export class MyLoggingHandler implements Middleware {
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

public async execute(context: Context): Promise<void> {
try {
Expand All @@ -132,7 +132,7 @@ export class MyLoggingHandler implements Middleware {
} else {
console.log(url);
}
await this.nextMiddleware.execute(context);
await this.nextMiddleware?.execute(context);
} catch (error) {
throw error;
}
Expand Down
39 changes: 10 additions & 29 deletions src/HTTPClientFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,8 @@

import { HTTPClient } from "./HTTPClient";
import { AuthenticationProvider } from "./IAuthenticationProvider";
import { AuthenticationHandler } from "./middleware/AuthenticationHandler";
import { HTTPMessageHandler } from "./middleware/HTTPMessageHandler";
import { Middleware } from "./middleware/IMiddleware";
import { RedirectHandlerOptions } from "./middleware/options/RedirectHandlerOptions";
import { RetryHandlerOptions } from "./middleware/options/RetryHandlerOptions";
import { RedirectHandler } from "./middleware/RedirectHandler";
import { RetryHandler } from "./middleware/RetryHandler";
import { TelemetryHandler } from "./middleware/TelemetryHandler";

/**
* @private
* To check whether the environment is node or not
* @returns A boolean representing the environment is node or not
*/
const isNodeEnvironment = (): boolean => {
return typeof process === "object" && typeof require === "function";
};
import { MiddlewareFactory } from "./middleware/MiddlewareFactory";

/**
* @class
Expand All @@ -48,21 +33,17 @@ export class HTTPClientFactory {
* them will remain same. For example, Retry and Redirect handlers might be working multiple times for a request based on the response but their auth token would remain same.
*/
public static createWithAuthenticationProvider(authProvider: AuthenticationProvider): HTTPClient {
const authenticationHandler = new AuthenticationHandler(authProvider);
const retryHandler = new RetryHandler(new RetryHandlerOptions());
const telemetryHandler = new TelemetryHandler();
const httpMessageHandler = new HTTPMessageHandler();
const middlewareChain = MiddlewareFactory.getDefaultMiddlewareChain(authProvider);

authenticationHandler.setNext(retryHandler);
if (isNodeEnvironment()) {
const redirectHandler = new RedirectHandler(new RedirectHandlerOptions());
retryHandler.setNext(redirectHandler);
redirectHandler.setNext(telemetryHandler);
} else {
retryHandler.setNext(telemetryHandler);
for (let i = 0; i < middlewareChain.length - 1; i++) {
const current = middlewareChain[i];
const next = middlewareChain[i + 1];
current.setNext?.(next);
}
telemetryHandler.setNext(httpMessageHandler);
return HTTPClientFactory.createWithMiddleware(authenticationHandler);

const chainHead = middlewareChain[0];

return HTTPClientFactory.createWithMiddleware(chainHead);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/middleware/AuthenticationHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class AuthenticationHandler implements Middleware {
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

/**
* @public
Expand Down Expand Up @@ -85,6 +85,7 @@ export class AuthenticationHandler implements Middleware {
delete context.options.headers[AuthenticationHandler.AUTHORIZATION_HEADER];
}
}
if (!this.nextMiddleware) return;
return await this.nextMiddleware.execute(context);
}

Expand Down
2 changes: 1 addition & 1 deletion src/middleware/ChaosHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class ChaosHandler implements Middleware {
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

/**
* @public
Expand Down
18 changes: 18 additions & 0 deletions src/middleware/HTTPMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ import { Middleware } from "./IMiddleware";
* Class for HTTPMessageHandler
*/
export class HTTPMessageHandler implements Middleware {
/**
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware?: Middleware;

/**
* @public
* @async
Expand All @@ -27,5 +33,17 @@ export class HTTPMessageHandler implements Middleware {
*/
public async execute(context: Context): Promise<void> {
context.response = await fetch(context.request, context.options);
if (!this.nextMiddleware) return;
return await this.nextMiddleware.execute(context);
}

/**
* @public
* To set the next middleware in the chain
* @param {Middleware} next - The middleware instance
* @returns Nothing
*/
public setNext(next: Middleware): void {
this.nextMiddleware = next;
}
}
2 changes: 1 addition & 1 deletion src/middleware/IMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ import { Context } from "../IContext";
*/
export interface Middleware {
execute: (context: Context) => Promise<void>;
setNext?: (middleware: Middleware) => void;
setNext: (middleware: Middleware) => void;
}
4 changes: 2 additions & 2 deletions src/middleware/RedirectHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class RedirectHandler implements Middleware {
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

/**
* @public
Expand Down Expand Up @@ -190,7 +190,7 @@ export class RedirectHandler implements Middleware {
* @returns A promise that resolves to nothing
*/
private async executeWithRedirect(context: Context, redirectCount: number, options: RedirectHandlerOptions): Promise<void> {
await this.nextMiddleware.execute(context);
await this.nextMiddleware?.execute(context);
const response = context.response;
if (redirectCount < options.maxRedirects && this.isRedirect(response) && this.hasLocationHeader(response) && options.shouldRedirect(response)) {
++redirectCount;
Expand Down
9 changes: 7 additions & 2 deletions src/middleware/RetryHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class RetryHandler implements Middleware {
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

/**
* @private
Expand Down Expand Up @@ -170,7 +170,10 @@ export class RetryHandler implements Middleware {
* @returns A Promise that resolves to nothing
*/
private async executeWithRetry(context: Context, retryAttempts: number, options: RetryHandlerOptions): Promise<void> {
await this.nextMiddleware.execute(context);
// The execute method ensures the nextMiddleware will exist.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await this.nextMiddleware!.execute(context);

if (retryAttempts < options.maxRetries && this.isRetry(context.response) && this.isBuffered(context.request, context.options) && options.shouldRetry(options.delay, retryAttempts, context.request, context.options, context.response)) {
++retryAttempts;
setRequestHeader(context.request, context.options, RetryHandler.RETRY_ATTEMPT_HEADER, retryAttempts.toString());
Expand All @@ -190,6 +193,8 @@ export class RetryHandler implements Middleware {
* @returns A Promise that resolves to nothing
*/
public async execute(context: Context): Promise<void> {
if (!this.nextMiddleware) return;

const retryAttempts = 0;
const options: RetryHandlerOptions = this.getOptions(context);
TelemetryHandlerOptions.updateFeatureUsageFlag(context, FeatureUsageFlag.RETRY_HANDLER_ENABLED);
Expand Down
3 changes: 2 additions & 1 deletion src/middleware/TelemetryHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class TelemetryHandler implements Middleware {
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware: Middleware;
private nextMiddleware?: Middleware;

/**
* @public
Expand Down Expand Up @@ -88,6 +88,7 @@ export class TelemetryHandler implements Middleware {
delete context.options.headers[TelemetryHandler.CLIENT_REQUEST_ID_HEADER];
delete context.options.headers[TelemetryHandler.SDK_VERSION_HEADER];
}
if (!this.nextMiddleware) return;
return await this.nextMiddleware.execute(context);
}

Expand Down
13 changes: 12 additions & 1 deletion test/DummyHTTPMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ export class DummyHTTPMessageHandler implements Middleware {
*/
private responses: Response[];

/**
* @private
* A member to hold next middleware in the middleware chain
*/
private nextMiddleware?: Middleware;

/**
* @public
* @constructor
Expand Down Expand Up @@ -54,6 +60,11 @@ export class DummyHTTPMessageHandler implements Middleware {
*/
public async execute(context: Context) {
context.response = this.responses.shift();
return;
if (!this.nextMiddleware) return;
return await this.nextMiddleware.execute(context);
}

public setNext(middleware: Middleware) {
this.nextMiddleware = middleware;
}
}