Skip to content
This repository has been archived by the owner on Oct 10, 2024. It is now read-only.

Remove node-fetch as a dependency #83

Closed
jwerre opened this issue Jun 7, 2024 · 3 comments
Closed

Remove node-fetch as a dependency #83

jwerre opened this issue Jun 7, 2024 · 3 comments

Comments

@jwerre
Copy link

jwerre commented Jun 7, 2024

I see that node-fetch is used to make request to the Mistral API. Since node-fetch doesn't support CommonJs it would be pretty easy to remove it as a dependency. And, even better, you'd have zero dependencies. All you need to do is import the native https module and replace _request with this:

import { request } from 'https';

async _request<T, U>(
	method: string,
	path: string,
	body?: U,
	retries: number = 0
): Promise<T> {
	const options = {
		method,
		port: 443,
		body: method !== 'get' && body ? JSON.stringify(body) : null,
		headers: {
			'User-Agent': `mistral/mistral-client`,
			Accept: 'application/json',
			'Content-Type': 'application/json',
			Authorization: `Bearer ${this.apiKey}`,
		},
	};
	const url = new URL(`${this.endpoint}/${path}`);

	return new Promise((resolve, reject) => {
		const req = request(url, options, (res) => {
			let data = '';

			res.on('data', (chunk) => {
				data += chunk;
			});

			res.on('end', () => {
				if (
					RETRY_STATUS_CODES.includes(res.statusCode ?? 500) &&
					retries < this.maxRetries
				) {
					console.debug(
						`Retry ${retries + 1}/${
							this.maxRetries
						} - Status Code: ${res.statusCode}`
					);
					setTimeout(() => {
						resolve(
							this._request<T, U>(
								method,
								path,
								body,
								retries + 1
							)
						);
					}, RETRY_DELAY_MS);
				} else {
					resolve(JSON.parse(data) as T);
				}
			});
		});

		req.on('error', (err) => {
			if (retries < this.maxRetries) {
				console.debug(
					`Retry ${retries + 1}/${this.maxRetries} - Error: ${
						err.message
					}`
				);
				setTimeout(() => {
					resolve(
						this._request<T, U>(method, path, body, retries + 1)
					);
				}, RETRY_DELAY_MS);
			} else {
				reject(new MistralAPIError(err.message));
			}
		});

		if (options.body) {
			req.write(options.body);
		}

		req.setTimeout(this.timeout, () => {
			req.destroy();
			if (retries < this.maxRetries) {
				console.log(
					`Retry ${retries + 1}/${this.maxRetries} - Timeout`
				);
				setTimeout(() => {
					resolve(
						this._request<T, U>(method, path, body, retries + 1)
					);
				}, RETRY_DELAY_MS);
			} else {
				reject(new MistralAPIError('Request timed out'));
			}
		});

		req.end();
	});
}

Happy to make a PR once #73 is merged

@alex-shortt
Copy link

Yeah had to make my own api wrapper, this was killing my builds. Was able to swap node-fetch with https://www.npmjs.com/package/node-fetch-commonjs

@jwerre
Copy link
Author

jwerre commented Jul 22, 2024

Yeah had to make my own api wrapper, this was killing my builds. Was able to swap node-fetch with https://www.npmjs.com/package/node-fetch-commonjs

You could try using Fetch v2

@GaspardBT
Copy link
Contributor

Thanks for reporting this issue. We have deprecated this package in favor of mistralai/client-ts, which is the new official Mistral client, compatible with both TypeScript and JavaScript.

You can find all installation information here.

This change is effective starting with version 1.0.0 of the npm package.

Let us know if your issue persists with the new package by opening an issue there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants