Skip to content

Refactor client tests, deprecate addProtocolIfNotPresent, fix abort bug, adjust documentation #1926

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

Open
wants to merge 3 commits into
base: main
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
60 changes: 31 additions & 29 deletions src/http-requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
MeiliSearchRequestError,
MeiliSearchRequestTimeOutError,
} from "./errors/index.js";
import { addProtocolIfNotPresent, addTrailingSlash } from "./utils.js";
import { addProtocolIfNotPresent } from "./utils.js";

/** Append a set of key value pairs to a {@link URLSearchParams} object. */
function appendRecordToURLSearchParams(
Expand All @@ -34,36 +34,32 @@ function appendRecordToURLSearchParams(
}
}

const AGENT_HEADER_KEY = "X-Meilisearch-Client";
const CONTENT_TYPE_KEY = "Content-Type";
const AUTHORIZATION_KEY = "Authorization";
const PACAKGE_AGENT = `Meilisearch JavaScript (v${PACKAGE_VERSION})`;

/**
* Creates a new Headers object from a {@link HeadersInit} and adds various
* properties to it, some from {@link Config}.
*
* @returns A new Headers object
* properties to it, as long as they're not already provided by the user.
*/
function getHeaders(config: Config, headersInit?: HeadersInit): Headers {
const agentHeader = "X-Meilisearch-Client";
const packageAgent = `Meilisearch JavaScript (v${PACKAGE_VERSION})`;
const contentType = "Content-Type";
const authorization = "Authorization";

const headers = new Headers(headersInit);

// do not override if user provided the header
if (config.apiKey && !headers.has(authorization)) {
headers.set(authorization, `Bearer ${config.apiKey}`);
if (config.apiKey && !headers.has(AUTHORIZATION_KEY)) {
headers.set(AUTHORIZATION_KEY, `Bearer ${config.apiKey}`);
}

if (!headers.has(contentType)) {
headers.set(contentType, "application/json");
if (!headers.has(CONTENT_TYPE_KEY)) {
headers.set(CONTENT_TYPE_KEY, "application/json");
}

// Creates the custom user agent with information on the package used.
if (config.clientAgents !== undefined) {
const clients = config.clientAgents.concat(packageAgent);

headers.set(agentHeader, clients.join(" ; "));
const agents = config.clientAgents.concat(PACAKGE_AGENT);
headers.set(AGENT_HEADER_KEY, agents.join(" ; "));
} else {
headers.set(agentHeader, packageAgent);
headers.set(AGENT_HEADER_KEY, PACAKGE_AGENT);
}

return headers;
Expand All @@ -85,19 +81,23 @@ const TIMEOUT_ID = {};
* function that clears the timeout
*/
function getTimeoutFn(
requestInit: RequestInit,
init: RequestInit,
ms: number,
): () => (() => void) | void {
const { signal } = requestInit;
const { signal } = init;
const ac = new AbortController();

init.signal = ac.signal;

if (signal != null) {
let acSignalFn: (() => void) | null = null;

if (signal.aborted) {
ac.abort(signal.reason);
} else {
const fn = () => ac.abort(signal.reason);
const fn = () => {
ac.abort(signal.reason);
};

signal.addEventListener("abort", fn, { once: true });

Expand All @@ -110,7 +110,9 @@ function getTimeoutFn(
return;
}

const to = setTimeout(() => ac.abort(TIMEOUT_ID), ms);
const to = setTimeout(() => {
ac.abort(TIMEOUT_ID);
}, ms);
const fn = () => {
clearTimeout(to);

Expand All @@ -128,10 +130,10 @@ function getTimeoutFn(
};
}

requestInit.signal = ac.signal;

return () => {
const to = setTimeout(() => ac.abort(TIMEOUT_ID), ms);
const to = setTimeout(() => {
ac.abort(TIMEOUT_ID);
}, ms);
return () => clearTimeout(to);
};
}
Expand All @@ -144,10 +146,10 @@ export class HttpRequests {
#requestTimeout?: Config["timeout"];

constructor(config: Config) {
const host = addTrailingSlash(addProtocolIfNotPresent(config.host));
const host = addProtocolIfNotPresent(config.host);

try {
this.#url = new URL(host);
this.#url = new URL(host.endsWith("/") ? host : host + "/");
} catch (error) {
throw new MeiliSearchError("The provided host is not valid", {
cause: error,
Expand Down Expand Up @@ -177,8 +179,8 @@ export class HttpRequests {

const headers = new Headers(extraHeaders);

if (contentType !== undefined && !headers.has("Content-Type")) {
headers.set("Content-Type", contentType);
if (contentType !== undefined && !headers.has(CONTENT_TYPE_KEY)) {
headers.set(CONTENT_TYPE_KEY, contentType);
}

for (const [key, val] of this.#requestInit.headers) {
Expand Down
2 changes: 1 addition & 1 deletion src/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class TaskClient {
}
}
} catch (error) {
throw Object.is((error as Error).cause, TIMEOUT_ID)
throw Object.is((error as Error)?.cause, TIMEOUT_ID)
? new MeiliSearchTaskTimeOutError(taskUid, timeout)
: error;
}
Expand Down
22 changes: 12 additions & 10 deletions src/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,7 @@ export type HttpRequestsRequestInit = Omit<BaseRequestInit, "headers"> & {

/** Main configuration object for the meilisearch client. */
export type Config = {
/**
* The base URL for reaching a meilisearch instance.
*
* @remarks
* Protocol and trailing slash can be omitted.
*/
/** The base URL for reaching a meilisearch instance. */
host: string;
/**
* API key for interacting with a meilisearch instance.
Expand All @@ -59,8 +54,8 @@ export type Config = {
*/
apiKey?: string;
/**
* Custom strings that will be concatted to the "X-Meilisearch-Client" header
* on each request.
* Custom strings that will be concatenated to the "X-Meilisearch-Client"
* header on each request.
*/
clientAgents?: string[];
/** Base request options that may override the default ones. */
Expand All @@ -69,12 +64,19 @@ export type Config = {
* Custom function that can be provided in place of {@link fetch}.
*
* @remarks
* API response errors will have to be handled manually with this as well.
* API response errors have to be handled manually.
* @deprecated This will be removed in a future version. See
* {@link https://github.com/meilisearch/meilisearch-js/issues/1824 | issue}.
*/
httpClient?: (...args: Parameters<typeof fetch>) => Promise<unknown>;
/** Timeout in milliseconds for each HTTP request. */
/**
* Timeout in milliseconds for each HTTP request.
*
* @remarks
* This uses {@link setTimeout}, which is not guaranteed to respect the
* provided milliseconds accurately.
* @see {@link https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout#reasons_for_delays_longer_than_specified}
*/
timeout?: number;
defaultWaitOptions?: WaitOptions;
};
Expand Down
20 changes: 11 additions & 9 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
return await new Promise((resolve) => setTimeout(resolve, ms));
}

let warningDispatched = false;
function addProtocolIfNotPresent(host: string): string {
if (!(host.startsWith("https://") || host.startsWith("http://"))) {
return `http://${host}`;
if (/^https?:\/\//.test(host)) {
return host;
}
return host;
}

function addTrailingSlash(url: string): string {
if (!url.endsWith("/")) {
url += "/";
if (!warningDispatched) {
console.warn(
`DEPRECATION WARNING: missing protocol in provided host ${host} will no longer be supported in the future`,
);
warningDispatched = true;

Check warning on line 15 in src/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/utils.ts#L11-L15

Added lines #L11 - L15 were not covered by tests
}
return url;

return `http://${host}`;

Check warning on line 18 in src/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/utils.ts#L18

Added line #L18 was not covered by tests
}

export { sleep, addProtocolIfNotPresent, addTrailingSlash };
export { sleep, addProtocolIfNotPresent };
Loading