Skip to content

Commit

Permalink
network: Create proxyClient to handle errors on API calls done via …
Browse files Browse the repository at this point in the history
…the proxy API (#3046)

<!--- TITLE FORMAT: "component: short description", e.g. "k8s: add pod
log reader" -->

### Description
API calls done via the proxy API always return 200 unless there's a 5XX
error. There's a `httpStatus` property that's returned inside the `data`
property on Axios calls which contains the actual HTTP status.

The new `proxyClient` now will be able to handle those errors correctly.

<!-- Reference previous related pull requests below. -->

<!-- [OPTIONAL] Include screenshots below for frontend changes. -->

### Testing Performed
Unit testing and tested on internal UI.
  • Loading branch information
ajimenezlyft committed Jun 21, 2024
1 parent 02a7cac commit 74de58c
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 10 deletions.
21 changes: 20 additions & 1 deletion frontend/packages/core/src/Network/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,25 @@ const grpcResponseToError = (clientError: AxiosError): ClutchError => {
return error;
};

const HTTP_CODE_MAPPING = {
200: "OK",
400: "Failed Precondition",
401: "Unauthenticated",
403: "Permission Denied",
404: "Not Found",
409: "Already Exists",
429: "Resource Exhausted",
499: "Cancelled",
500: "Internal Server Error",
501: "Not Implemented",
503: "Service Unavailable",
504: "Gateway Timeout",
};

const httpCodeToText = (code: number): string => {
return HTTP_CODE_MAPPING[code] || "Unknown";
};

/* eslint-disable no-underscore-dangle */
const isHelpDetails = (details: ErrorDetails): details is Help => {
return details._type === "types.googleapis.com/google.rpc.Help";
Expand All @@ -291,4 +310,4 @@ const isClutchErrorDetails = (details: ErrorDetails): details is IClutch.api.v1.
};
/* eslint-enable */

export { grpcResponseToError, isClutchErrorDetails, isHelpDetails };
export { grpcResponseToError, httpCodeToText, isClutchErrorDetails, isHelpDetails };
45 changes: 39 additions & 6 deletions frontend/packages/core/src/Network/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { AxiosError, AxiosResponse } from "axios";
import axios from "axios";

import type { ClutchError } from "./errors";
import { grpcResponseToError } from "./errors";
import { grpcResponseToError, httpCodeToText } from "./errors";

/**
* HTTP response status.
Expand All @@ -21,10 +21,34 @@ export interface HttpStatus {
text: string;
}

interface CreateClientProps {
response?: {
success: (response: AxiosResponse) => AxiosResponse | Promise<never>;
error: (error: AxiosError) => Promise<ClutchError>;
};
}

const successInterceptor = (response: AxiosResponse) => {
return response;
};

const successProxyInterceptor = (response: AxiosResponse) => {
// Handle proxy errors
if (response.data.httpStatus >= 400) {
const error = {
status: {
code: response.data.httpStatus,
text: httpCodeToText(response.data.httpStatus),
},
message: response.data.response.message,
data: response.data,
} as ClutchError;
return Promise.reject(error);
}

return response;
};

const errorInterceptor = (error: AxiosError): Promise<ClutchError> => {
const response = error?.response;
if (response === undefined) {
Expand All @@ -49,7 +73,7 @@ const errorInterceptor = (error: AxiosError): Promise<ClutchError> => {
// since we have already accounted for axios errors.
const responseData = error?.response?.data;
// if the response data has a code on it we know it's a gRPC response.
let err;
let err: ClutchError;
if (responseData?.code !== undefined) {
err = grpcResponseToError(error);
} else {
Expand All @@ -69,18 +93,27 @@ const errorInterceptor = (error: AxiosError): Promise<ClutchError> => {
return Promise.reject(err);
};

const createClient = () => {
const createClient = ({ response }: CreateClientProps) => {
const axiosClient = axios.create({
// n.b. the client will treat any response code >= 400 as an error and apply the error interceptor.
validateStatus: status => {
return status < 400;
},
});
axiosClient.interceptors.response.use(successInterceptor, errorInterceptor);

if (response) {
axiosClient.interceptors.response.use(response.success, response.error);
}

return axiosClient;
};

const client = createClient();
const client = createClient({
response: { success: successInterceptor, error: errorInterceptor },
});

const proxyClient = createClient({
response: { success: successProxyInterceptor, error: errorInterceptor },
});

export { client, errorInterceptor, successInterceptor };
export { client, proxyClient, errorInterceptor, successInterceptor, successProxyInterceptor };
33 changes: 31 additions & 2 deletions frontend/packages/core/src/Network/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { AxiosError } from "axios";
import type { AxiosError, AxiosResponse } from "axios";

import type { ClutchError } from "../errors";
import { client, errorInterceptor } from "../index";
import { client, errorInterceptor, successProxyInterceptor } from "../index";

describe("error interceptor", () => {
describe("on axios error", () => {
Expand Down Expand Up @@ -151,3 +151,32 @@ describe("axios client", () => {
expect(client.defaults.validateStatus(399)).toBe(true);
});
});

describe("axios success proxy interceptor", () => {
it("returns an error if the proxy call returns response.data.httpStatus >= 400", async () => {
const response = {
status: 200,
statusText: "Not Found",
data: {
httpStatus: 404,
headers: {
"Cache-Control": ["no-cache"],
},
response: {
message: "Item not found",
},
},
} as AxiosResponse;

const err = {
status: {
code: 404,
text: "Not Found",
},
message: "Item not found",
data: response.data,
} as ClutchError;

await expect(successProxyInterceptor(response)).rejects.toEqual(err);
});
});
2 changes: 1 addition & 1 deletion frontend/packages/core/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export {
useParams,
useSearchParams,
} from "./navigation";
export { client } from "./Network";
export { client, proxyClient } from "./Network";
export * from "./NPS";
export { default as ExpansionPanel } from "./panel";
export { default as Paper } from "./paper";
Expand Down

0 comments on commit 74de58c

Please sign in to comment.