Skip to content

Commit

Permalink
fix: remove use undici for fetch function (#470)
Browse files Browse the repository at this point in the history
## Description

Changes the fetch function to use undici fetch. Updates the k8sCfg to
return undici RequestInit instead of node-fetch RequestInit.

**Note:** - Do not merge is present on this PR because we are waiting
for December to release this as it is a big change to the surface area
of KFC and also affects Pepr. This PR removes the remaining node-fetch
from Pepr

## Related Issue

Fixes #459

<!-- or -->

Relates to #

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging

- [ ] Test, docs, adr added or updated as needed
- [ ] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Signed-off-by: Case Wylie <[email protected]>
  • Loading branch information
cmwylie19 authored Dec 2, 2024
1 parent 01f6579 commit 015e9e5
Show file tree
Hide file tree
Showing 7 changed files with 298 additions and 183 deletions.
164 changes: 116 additions & 48 deletions src/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,59 +1,108 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Kubernetes Fluent Client Authors

import { expect, test, beforeEach } from "@jest/globals";

import { expect, test, beforeEach, afterEach } from "@jest/globals";
import { StatusCodes } from "http-status-codes";
import nock from "nock";
import { RequestInit } from "node-fetch";
import { RequestInit } from "undici";
import { fetch } from "./fetch";

import { MockAgent, setGlobalDispatcher } from "undici";

let mockAgent: MockAgent;
interface Todo {
userId: number;
id: number;
title: string;
completed: boolean;
}
beforeEach(() => {
nock("https://jsonplaceholder.typicode.com")
.get("/todos/1")
.reply(200, {
mockAgent = new MockAgent();
setGlobalDispatcher(mockAgent);
mockAgent.disableNetConnect();

const mockClient = mockAgent.get("https://jsonplaceholder.typicode.com");

mockClient.intercept({ path: "/todos/1", method: "GET" }).reply(
StatusCodes.OK,
{
userId: 1,
id: 1,
title: "Example title",
completed: false,
})
.post("/todos", {
title: "test todo",
userId: 1,
completed: false,
})
.reply(200, (uri, requestBody) => requestBody)
.get("/todos/empty-null")
.reply(200, undefined)
.get("/todos/empty-string")
.reply(200, "")
.get("/todos/empty-object")
.reply(200, {})
.get("/todos/invalid")
.replyWithError("Something bad happened");
},
{
headers: {
"Content-Type": "application/json; charset=utf-8",
},
},
);

mockClient.intercept({ path: "/todos", method: "POST" }).reply(
StatusCodes.OK,
{ title: "test todo", userId: 1, completed: false },
{
headers: {
"Content-Type": "application/json; charset=utf-8",
},
},
);

mockClient
.intercept({ path: "/todos/empty-null", method: "GET" })
.reply(StatusCodes.OK, undefined);

mockClient.intercept({ path: "/todos/empty-string", method: "GET" }).reply(StatusCodes.OK, "");

mockClient.intercept({ path: "/todos/empty-object", method: "GET" }).reply(
StatusCodes.OK,
{},
{
headers: {
"Content-Type": "application/json; charset=utf-8",
},
},
);

mockClient
.intercept({ path: "/todos/invalid", method: "GET" })
.replyWithError(new Error("Something bad happened"));
});

afterEach(async () => {
try {
await mockAgent.close();
} catch (error) {
console.error("Error closing mock agent", error);
}
});

test("fetch: should return without type data", async () => {
const url = "https://jsonplaceholder.typicode.com/todos/1";
const { data, ok } = await fetch<{ title: string }>(url);
const requestOptions: RequestInit = {
method: "GET",
headers: {
hi: "there",
"content-type": "application/json; charset=UTF-8",
},
};
const { data, ok } = await fetch<Todo>(url, requestOptions);
expect(ok).toBe(true);
expect(data["title"]).toBe("Example title");
expect(data.title).toBe("Example title");
});

test("fetch: should return parsed JSON response as a specific type", async () => {
interface Todo {
userId: number;
id: number;
title: string;
completed: boolean;
}

const url = "https://jsonplaceholder.typicode.com/todos/1";
const { data, ok } = await fetch<Todo>(url);
expect(ok).toBe(true);
expect(data.id).toBe(1);
expect(typeof data.title).toBe("string");
expect(typeof data.completed).toBe("boolean");
const requestOptions: RequestInit = {
method: "GET",
headers: {
"Content-Type": "application/json; charset=UTF-8",
},
};
const res = await fetch<Todo>(url, requestOptions);
expect(res.ok).toBe(true);

expect(res.data.id).toBe(1);
expect(typeof res.data.title).toBe("string");
expect(typeof res.data.completed).toBe("boolean");
});

test("fetch: should handle additional request options", async () => {
Expand All @@ -66,22 +115,18 @@ test("fetch: should handle additional request options", async () => {
completed: false,
}),
headers: {
"Content-type": "application/json; charset=UTF-8",
"Content-Type": "application/json; charset=UTF-8",
},
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { data, ok } = await fetch<any>(url, requestOptions);
expect(ok).toBe(true);
expect(data["title"]).toBe("test todo");
expect(data["userId"]).toBe(1);
expect(data["completed"]).toBe(false);
const res = await fetch<Todo>(url, requestOptions);
expect(res.ok).toBe(true);
expect(res.data).toStrictEqual({ title: "test todo", userId: 1, completed: false });
});

test("fetch: should handle empty (null) responses", async () => {
const url = "https://jsonplaceholder.typicode.com/todos/empty-null";
const resp = await fetch(url);

expect(resp.data).toBe("");
expect(resp.ok).toBe(true);
expect(resp.status).toBe(StatusCodes.OK);
Expand All @@ -90,16 +135,20 @@ test("fetch: should handle empty (null) responses", async () => {
test("fetch: should handle empty (string) responses", async () => {
const url = "https://jsonplaceholder.typicode.com/todos/empty-string";
const resp = await fetch(url);

expect(resp.data).toBe("");
expect(resp.ok).toBe(true);
expect(resp.status).toBe(StatusCodes.OK);
});

test("fetch: should handle empty (object) responses", async () => {
const url = "https://jsonplaceholder.typicode.com/todos/empty-object";
const resp = await fetch(url);

const requestOptions: RequestInit = {
method: "GET",
headers: {
"Content-Type": "application/json; charset=UTF-8",
},
};
const resp = await fetch(url, requestOptions);
expect(resp.data).toEqual({});
expect(resp.ok).toBe(true);
expect(resp.status).toBe(StatusCodes.OK);
Expand All @@ -113,3 +162,22 @@ test("fetch: should handle failed requests without throwing an error", async ()
expect(resp.ok).toBe(false);
expect(resp.status).toBe(StatusCodes.BAD_REQUEST);
});

test("fetch wrapper respects MockAgent", async () => {
const mockClient = mockAgent.get("https://example.com");

mockClient.intercept({ path: "/test", method: "GET" }).reply(
200,
{ success: true },
{
headers: {
"Content-Type": "application/json; charset=utf-8",
},
},
);

const response = await fetch<{ success: boolean }>("https://example.com/test");

expect(response.ok).toBe(true);
expect(response.data).toEqual({ success: true });
});
23 changes: 7 additions & 16 deletions src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-FileCopyrightText: 2023-Present The Kubernetes Fluent Client Authors

import { StatusCodes } from "http-status-codes";
import fetchRaw, { FetchError, RequestInfo, RequestInit } from "node-fetch";
import { fetch as undiciFetch, RequestInfo, RequestInit } from "undici";

export type FetchResponse<T> = {
data: T;
Expand Down Expand Up @@ -30,12 +30,12 @@ export async function fetch<T>(
): Promise<FetchResponse<T>> {
let data = undefined as unknown as T;
try {
const resp = await fetchRaw(url, init);
const resp = await undiciFetch(url, init);
const contentType = resp.headers.get("content-type") || "";

// Parse the response as JSON if the content type is JSON
if (contentType.includes("application/json")) {
data = await resp.json();
data = (await resp.json()) as T;
} else {
// Otherwise, return however the response was read
data = (await resp.text()) as unknown as T;
Expand All @@ -48,23 +48,14 @@ export async function fetch<T>(
statusText: resp.statusText,
};
} catch (e) {
if (e instanceof FetchError) {
// Parse the error code from the FetchError or default to 400 (Bad Request)
const status = parseInt(e.code || "400");

return {
data,
ok: false,
status,
statusText: e.message,
};
}
const status = parseInt(e?.code) || StatusCodes.BAD_REQUEST;
const statusText = e?.message || "Unknown error";

return {
data,
ok: false,
status: StatusCodes.BAD_REQUEST,
statusText: "Unknown error",
status,
statusText,
};
}
}
8 changes: 7 additions & 1 deletion src/fluent/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@
import { KubernetesListObject, KubernetesObject } from "@kubernetes/client-node";
import { Operation } from "fast-json-patch";
import type { PartialDeep } from "type-fest";

import { RequestInit as UndiciRequestInit } from "undici";
import { GenericClass, GroupVersionKind } from "../types";
import { WatchCfg, Watcher } from "./watch";
import https from "https";
import { SecureClientSessionOptions } from "http2";

/**
* Fetch options and server URL
*/
export type K8sConfigPromise = Promise<{ opts: UndiciRequestInit; serverUrl: string | URL }>;

/**
* Agent options for the the http2Watch
*/
Expand Down
Loading

0 comments on commit 015e9e5

Please sign in to comment.