Remove custom TypeScript ESLint rules and adapt code, fix HttpRequests issue where timeout errors sometimes weren't properly thrown (#1749)

* const comments ans unsafe calls removed

* cast some any's

* removed and fixed no-unsafe-member-access

* removed no unsafe args, and cast some unsafe ones

* adjusted all no-unsafe-member-access

* removed floating promises

* removed and adjusted unsafe returns and unsafe assignenments

* fixed up unused vars

* removed explicit any's

* changed void to promise.all for unawaited promises

* correctly casting error and put back the eslint exception so the client builds with ts

* updated node-ts test

* Merge with main, and reset

* Remove all unnecessary ts eslint settings, adapt code

* Convert Array types to simple [] type

* Fix assertion being in the wrong place

* Fix unhandled rejection

* Revert api error changes

* Apply fix for abort errors not beign caught from response.text()

---------

Co-authored-by: F. Levi <[email protected]>

Author: Barabasbalazs

eslint.config.js

@@ -29,30 +29,12 @@ export default tseslint.config([
       },
     },
     rules: {
-      // TODO: Remove the ones between "~~", adapt code
-      // ~~
-      "@typescript-eslint/prefer-as-const": "off",
-      "@typescript-eslint/ban-ts-comment": "off",
-      "@typescript-eslint/no-unsafe-call": "off",
-      "@typescript-eslint/no-unsafe-member-access": "off",
-      "@typescript-eslint/no-unsafe-return": "off",
-      "@typescript-eslint/no-unsafe-assignment": "off",
-      "@typescript-eslint/no-unsafe-argument": "off",
-      "@typescript-eslint/no-floating-promises": "off",
-      // ~~
-      "@typescript-eslint/array-type": ["warn", { default: "array-simple" }],
-      // TODO: Should be careful with this rule, should leave it be and disable
-      //       it within files where necessary with explanations
-      "@typescript-eslint/no-explicit-any": "off",
       "@typescript-eslint/no-unused-vars": [
         "error",
         // argsIgnorePattern: https://eslint.org/docs/latest/rules/no-unused-vars#argsignorepattern
         // varsIgnorePattern: https://eslint.org/docs/latest/rules/no-unused-vars#varsignorepattern
         { args: "all", argsIgnorePattern: "^_", varsIgnorePattern: "^_" },
       ],
-      // TODO: Not recommended to disable rule, should instead disable locally
-      //       with explanation
-      "@typescript-eslint/ban-ts-ignore": "off",
     },
   },
   // Vitest

src/http-requests.ts

@@ -4,6 +4,7 @@ import type {
   RequestOptions,
   MainRequestOptions,
   URLSearchParamsRecord,
+  MeiliSearchErrorResponse,
 } from "./types.js";
 import { PACKAGE_VERSION } from "./package-version.js";
 import {
@@ -67,10 +68,9 @@ function getHeaders(config: Config, headersInit?: HeadersInit): Headers {
   return headers;
 }
 
-// This could be a symbol, but Node.js 18 fetch doesn't support that yet
-// and it might just go EOL before it ever does.
-// https://github.com/nodejs/node/issues/49557
-const TIMEOUT_OBJECT = {};
+// TODO: Convert to Symbol("timeout id") when Node.js 18 is dropped
+/** Used to identify whether an error is a timeout error after fetch request. */
+const TIMEOUT_ID = {};
 
 /**
  * Attach a timeout signal to a {@link RequestInit}, while preserving original
@@ -109,7 +109,7 @@ function getTimeoutFn(
         return;
       }
 
-      const to = setTimeout(() => ac.abort(TIMEOUT_OBJECT), ms);
+      const to = setTimeout(() => ac.abort(TIMEOUT_ID), ms);
       const fn = () => {
         clearTimeout(to);
 
@@ -130,7 +130,7 @@ function getTimeoutFn(
   requestInit.signal = ac.signal;
 
   return () => {
-    const to = setTimeout(() => ac.abort(TIMEOUT_OBJECT), ms);
+    const to = setTimeout(() => ac.abort(TIMEOUT_ID), ms);
     return () => clearTimeout(to);
   };
 }
@@ -208,7 +208,7 @@ export class HttpRequests {
       appendRecordToURLSearchParams(url.searchParams, params);
     }
 
-    const requestInit: RequestInit = {
+    const init: RequestInit = {
       method,
       body:
         contentType === undefined || typeof body !== "string"
@@ -221,51 +221,44 @@ export class HttpRequests {
 
     const startTimeout =
       this.#requestTimeout !== undefined
-        ? getTimeoutFn(requestInit, this.#requestTimeout)
+        ? getTimeoutFn(init, this.#requestTimeout)
         : null;
 
-    const getResponseAndHandleErrorAndTimeout = <
-      U extends ReturnType<NonNullable<Config["httpClient"]> | typeof fetch>,
-    >(
-      responsePromise: U,
-      stopTimeout?: ReturnType<NonNullable<typeof startTimeout>>,
-    ) =>
-      responsePromise
-        .catch((error: unknown) => {
-          throw new MeiliSearchRequestError(
-            url.toString(),
-            Object.is(error, TIMEOUT_OBJECT)
-              ? new Error(`request timed out after ${this.#requestTimeout}ms`, {
-                  cause: requestInit,
-                })
-              : error,
-          );
-        })
-        .finally(() => stopTimeout?.()) as U;
-
     const stopTimeout = startTimeout?.();
 
-    if (this.#customRequestFn !== undefined) {
-      const response = await getResponseAndHandleErrorAndTimeout(
-        this.#customRequestFn(url, requestInit),
-        stopTimeout,
-      );
+    let response: Response;
+    let responseBody: string;
+    try {
+      if (this.#customRequestFn !== undefined) {
+        // When using a custom HTTP client, the response should already be handled and ready to be returned
+        return (await this.#customRequestFn(url, init)) as T;
+      }
 
-      // When using a custom HTTP client, the response should already be handled and ready to be returned
-      return response as T;
+      response = await fetch(url, init);
+      responseBody = await response.text();
+    } catch (error) {
+      throw new MeiliSearchRequestError(
+        url.toString(),
+        Object.is(error, TIMEOUT_ID)
+          ? new Error(`request timed out after ${this.#requestTimeout}ms`, {
+              cause: init,
+            })
+          : error,
+      );
+    } finally {
+      stopTimeout?.();
     }
 
-    const response = await getResponseAndHandleErrorAndTimeout(
-      fetch(url, requestInit),
-      stopTimeout,
-    );
-
-    const responseBody = await response.text();
     const parsedResponse =
-      responseBody === "" ? undefined : JSON.parse(responseBody);
+      responseBody === ""
+        ? undefined
+        : (JSON.parse(responseBody) as T | MeiliSearchErrorResponse);
 
     if (!response.ok) {
-      throw new MeiliSearchApiError(response, parsedResponse);
+      throw new MeiliSearchApiError(
+        response,
+        parsedResponse as MeiliSearchErrorResponse | undefined,
+      );
     }
 
     return parsedResponse as T;

src/indexes.ts

@@ -52,12 +52,13 @@ import type {
   EnqueuedTaskObject,
   ExtraRequestInit,
   PrefixSearch,
+  RecordAny,
 } from "./types.js";
 import { HttpRequests } from "./http-requests.js";
 import { Task, TaskClient } from "./task.js";
 import { EnqueuedTask } from "./enqueued-task.js";
 
-class Index<T extends Record<string, any> = Record<string, any>> {
+class Index<T extends RecordAny = RecordAny> {
   uid: string;
   primaryKey: string | undefined;
   createdAt: Date | undefined;
@@ -89,10 +90,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    * @param config - Additional request configuration options
    * @returns Promise containing the search response
    */
-  async search<
-    D extends Record<string, any> = T,
-    S extends SearchParams = SearchParams,
-  >(
+  async search<D extends RecordAny = T, S extends SearchParams = SearchParams>(
     query?: string | null,
     options?: S,
     extraRequestInit?: ExtraRequestInit,
@@ -113,7 +111,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    * @returns Promise containing the search response
    */
   async searchGet<
-    D extends Record<string, any> = T,
+    D extends RecordAny = T,
     S extends SearchParams = SearchParams,
   >(
     query?: string | null,
@@ -175,7 +173,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    * @returns Promise containing the search response
    */
   async searchSimilarDocuments<
-    D extends Record<string, any> = T,
+    D extends RecordAny = T,
     S extends SearchParams = SearchParams,
   >(params: SearchSimilarDocumentsParams): Promise<SearchResponse<D, S>> {
     return await this.httpRequest.post<SearchResponse<D, S>>({
@@ -357,7 +355,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    *   the `filter` field only available in Meilisearch v1.2 and newer
    * @returns Promise containing the returned documents
    */
-  async getDocuments<D extends Record<string, any> = T>(
+  async getDocuments<D extends RecordAny = T>(
     params?: DocumentsQuery<D>,
   ): Promise<ResourceResults<D[]>> {
     const relativeBaseURL = `indexes/${this.uid}/documents`;
@@ -384,7 +382,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    * @param parameters - Parameters applied on a document
    * @returns Promise containing Document response
    */
-  async getDocument<D extends Record<string, any> = T>(
+  async getDocument<D extends RecordAny = T>(
     documentId: string | number,
     parameters?: DocumentQuery<T>,
   ): Promise<D> {
@@ -476,7 +474,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    * @returns Promise containing an EnqueuedTask
    */
   async updateDocuments(
-    documents: Array<Partial<T>>,
+    documents: Partial<T>[],
     options?: DocumentOptions,
   ): Promise<EnqueuedTask> {
     const task = await this.httpRequest.put<EnqueuedTaskObject>({
@@ -497,7 +495,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
    * @returns Promise containing array of enqueued task objects for each batch
    */
   async updateDocumentsInBatches(
-    documents: Array<Partial<T>>,
+    documents: Partial<T>[],
     batchSize = 1000,
     options?: DocumentOptions,
   ): Promise<EnqueuedTask[]> {

src/meilisearch.ts

@@ -29,12 +29,14 @@ import type {
   BatchesQuery,
   MultiSearchResponseOrSearchResponse,
   Network,
+  RecordAny,
 } from "./types.js";
 import { ErrorStatusCode } from "./types.js";
 import { HttpRequests } from "./http-requests.js";
 import { TaskClient } from "./task.js";
 import { EnqueuedTask } from "./enqueued-task.js";
 import { type Batch, BatchClient } from "./batch.js";
+import type { MeiliSearchApiError } from "./errors/meilisearch-api-error.js";
 
 export class MeiliSearch {
   config: Config;
@@ -60,9 +62,7 @@ export class MeiliSearch {
    * @param indexUid - The index UID
    * @returns Instance of Index
    */
-  index<T extends Record<string, any> = Record<string, any>>(
-    indexUid: string,
-  ): Index<T> {
+  index<T extends RecordAny = RecordAny>(indexUid: string): Index<T> {
     return new Index<T>(this.config, indexUid);
   }
 
@@ -73,7 +73,7 @@ export class MeiliSearch {
    * @param indexUid - The index UID
    * @returns Promise returning Index instance
    */
-  async getIndex<T extends Record<string, any> = Record<string, any>>(
+  async getIndex<T extends RecordAny = RecordAny>(
     indexUid: string,
   ): Promise<Index<T>> {
     return new Index<T>(this.config, indexUid).fetchInfo();
@@ -170,10 +170,14 @@ export class MeiliSearch {
     try {
       await this.deleteIndex(uid);
       return true;
-    } catch (e: any) {
-      if (e.code === ErrorStatusCode.INDEX_NOT_FOUND) {
+    } catch (e) {
+      if (
+        (e as MeiliSearchApiError)?.cause?.code ===
+        ErrorStatusCode.INDEX_NOT_FOUND
+      ) {
         return false;
       }
+
       throw e;
     }
   }
@@ -244,7 +248,7 @@ export class MeiliSearch {
    */
   async multiSearch<
     T1 extends MultiSearchParams | FederatedMultiSearchParams,
-    T2 extends Record<string, any> = Record<string, any>,
+    T2 extends RecordAny = RecordAny,
   >(
     queries: T1,
     extraRequestInit?: ExtraRequestInit,

src/token.ts

@@ -1,5 +1,9 @@
 import type { webcrypto } from "node:crypto";
-import type { TenantTokenGeneratorOptions, TokenSearchRules } from "./types.js";
+import type {
+  TenantTokenGeneratorOptions,
+  TenantTokenHeader,
+  TokenClaims,
+} from "./types.js";
 
 function getOptionsWithDefaults(options: TenantTokenGeneratorOptions) {
   const {
@@ -80,20 +84,10 @@ async function sign(
 function getHeader({
   algorithm: alg,
 }: TenantTokenGeneratorOptionsWithDefaults): string {
-  const header = { alg, typ: "JWT" };
+  const header: TenantTokenHeader = { alg, typ: "JWT" };
   return encodeToBase64(header).replace(/=/g, "");
 }
 
-/**
- * @see {@link https://www.meilisearch.com/docs/learn/security/tenant_token_reference | Tenant token payload reference}
- * @see {@link https://github.com/meilisearch/meilisearch/blob/b21d7aedf9096539041362d438e973a18170f3fc/crates/meilisearch/src/extractors/authentication/mod.rs#L334-L340 | GitHub source code}
- */
-type TokenClaims = {
-  searchRules: TokenSearchRules;
-  exp?: number;
-  apiKeyUid: string;
-};
-
 /** Create the payload of the token. */
 function getPayload({
   searchRules,
@@ -124,8 +118,8 @@ function getPayload({
  * is the recommended way according to
  * {@link https://min-common-api.proposal.wintercg.org/#navigator-useragent-requirements | WinterCG specs}.
  * {@link https://developer.mozilla.org/en-US/docs/Web/API/Navigator/userAgent | User agent }
- * can be spoofed, `process` can be patched. It should prevent misuse for the
- * overwhelming majority of cases.
+ * can be spoofed, `process` can be patched. Even so it should prevent misuse
+ * for the overwhelming majority of cases.
  */
 function tryDetectEnvironment(): void {
   if (typeof navigator !== "undefined" && "userAgent" in navigator) {