Skip to content

Commit e84ad1c

Browse files
stainless-botRobertCraigie
authored andcommitted
chore(client): improve node-fetch file upload errors
chore: unknown commit message
1 parent 98e76bb commit e84ad1c

File tree

7 files changed

+123
-42
lines changed

7 files changed

+123
-42
lines changed

src/internal/uploads.ts

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type RequestOptions } from './request-options';
2-
import { type FilePropertyBag } from './builtin-types';
2+
import type { FilePropertyBag, Fetch } from './builtin-types';
33
import { isFsReadStreamLike, type FsReadStreamLike } from './shims';
4+
import type { OpenAI } from '../client';
45
import './polyfill/file.node.js';
56

67
type BlobLikePart = string | ArrayBuffer | ArrayBufferView | BlobLike | DataView;
@@ -203,21 +204,65 @@ const isAsyncIterableIterator = (value: any): value is AsyncIterableIterator<unk
203204
* Returns a multipart/form-data request if any part of the given request body contains a File / Blob value.
204205
* Otherwise returns the request as is.
205206
*/
206-
export const maybeMultipartFormRequestOptions = async (opts: RequestOptions): Promise<RequestOptions> => {
207+
export const maybeMultipartFormRequestOptions = async (
208+
opts: RequestOptions,
209+
fetch: OpenAI | Fetch,
210+
): Promise<RequestOptions> => {
207211
if (!hasUploadableValue(opts.body)) return opts;
208212

209-
return { ...opts, body: await createForm(opts.body) };
213+
return { ...opts, body: await createForm(opts.body, fetch) };
210214
};
211215

212216
type MultipartFormRequestOptions = Omit<RequestOptions, 'body'> & { body: unknown };
213217

214218
export const multipartFormRequestOptions = async (
215219
opts: MultipartFormRequestOptions,
220+
fetch: OpenAI | Fetch,
216221
): Promise<RequestOptions> => {
217-
return { ...opts, body: await createForm(opts.body) };
222+
return { ...opts, body: await createForm(opts.body, fetch) };
218223
};
219224

220-
export const createForm = async <T = Record<string, unknown>>(body: T | undefined): Promise<FormData> => {
225+
const supportsFormDataMap = new WeakMap<Fetch, Promise<boolean>>();
226+
227+
/**
228+
* node-fetch doesn't support the global FormData object in recent node versions. Instead of sending
229+
* properly-encoded form data, it just stringifies the object, resulting in a request body of "[object FormData]".
230+
* This function detects if the fetch function provided supports the global FormData object to avoid
231+
* confusing error messages later on.
232+
*/
233+
function supportsFormData(fetchObject: OpenAI | Fetch): Promise<boolean> {
234+
const fetch: Fetch = typeof fetchObject === 'function' ? fetchObject : (fetchObject as any).fetch;
235+
const cached = supportsFormDataMap.get(fetch);
236+
if (cached) return cached;
237+
const promise = (async () => {
238+
try {
239+
const FetchResponse = (
240+
'Response' in fetch ?
241+
fetch.Response
242+
: (await fetch('data:,')).constructor) as typeof Response;
243+
const data = new FormData();
244+
if (data.toString() === (await new FetchResponse(data).text())) {
245+
return false;
246+
}
247+
return true;
248+
} catch {
249+
// avoid false negatives
250+
return true;
251+
}
252+
})();
253+
supportsFormDataMap.set(fetch, promise);
254+
return promise;
255+
}
256+
257+
export const createForm = async <T = Record<string, unknown>>(
258+
body: T | undefined,
259+
fetch: OpenAI | Fetch,
260+
): Promise<FormData> => {
261+
if (!(await supportsFormData(fetch))) {
262+
throw new TypeError(
263+
'The provided fetch function does not support file uploads with the current global FormData class.',
264+
);
265+
}
221266
const form = new FormData();
222267
await Promise.all(Object.entries(body || {}).map(([key, value]) => addFormValue(form, key, value)));
223268
return form;

src/resources/audio/transcriptions.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ export class Transcriptions extends APIResource {
2828
body: TranscriptionCreateParams,
2929
options?: RequestOptions,
3030
): APIPromise<TranscriptionCreateResponse | string> {
31-
return this._client.post('/audio/transcriptions', multipartFormRequestOptions({ body, ...options }));
31+
return this._client.post(
32+
'/audio/transcriptions',
33+
multipartFormRequestOptions({ body, ...options }, this._client),
34+
);
3235
}
3336
}
3437

src/resources/audio/translations.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ export class Translations extends APIResource {
2626
body: TranslationCreateParams,
2727
options?: RequestOptions,
2828
): APIPromise<TranslationCreateResponse | string> {
29-
return this._client.post('/audio/translations', multipartFormRequestOptions({ body, ...options }));
29+
return this._client.post(
30+
'/audio/translations',
31+
multipartFormRequestOptions({ body, ...options }, this._client),
32+
);
3033
}
3134
}
3235

src/resources/files.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class Files extends APIResource {
3434
* storage limits.
3535
*/
3636
create(body: FileCreateParams, options?: RequestOptions): APIPromise<FileObject> {
37-
return this._client.post('/files', multipartFormRequestOptions({ body, ...options }));
37+
return this._client.post('/files', multipartFormRequestOptions({ body, ...options }, this._client));
3838
}
3939

4040
/**

src/resources/images.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,20 @@ export class Images extends APIResource {
1111
* Creates a variation of a given image.
1212
*/
1313
createVariation(body: ImageCreateVariationParams, options?: RequestOptions): APIPromise<ImagesResponse> {
14-
return this._client.post('/images/variations', multipartFormRequestOptions({ body, ...options }));
14+
return this._client.post(
15+
'/images/variations',
16+
multipartFormRequestOptions({ body, ...options }, this._client),
17+
);
1518
}
1619

1720
/**
1821
* Creates an edited or extended image given an original image and a prompt.
1922
*/
2023
edit(body: ImageEditParams, options?: RequestOptions): APIPromise<ImagesResponse> {
21-
return this._client.post('/images/edits', multipartFormRequestOptions({ body, ...options }));
24+
return this._client.post(
25+
'/images/edits',
26+
multipartFormRequestOptions({ body, ...options }, this._client),
27+
);
2228
}
2329

2430
/**

src/resources/uploads/parts.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ export class Parts extends APIResource {
2121
* [complete the Upload](https://platform.openai.com/docs/api-reference/uploads/complete).
2222
*/
2323
create(uploadID: string, body: PartCreateParams, options?: RequestOptions): APIPromise<UploadPart> {
24-
return this._client.post(`/uploads/${uploadID}/parts`, multipartFormRequestOptions({ body, ...options }));
24+
return this._client.post(
25+
`/uploads/${uploadID}/parts`,
26+
multipartFormRequestOptions({ body, ...options }, this._client),
27+
);
2528
}
2629
}
2730

tests/form.test.ts

Lines changed: 52 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,62 +3,83 @@ import { toFile } from 'openai/uploads';
33

44
describe('form data validation', () => {
55
test('valid values do not error', async () => {
6-
await multipartFormRequestOptions({
7-
body: {
8-
foo: 'foo',
9-
string: 1,
10-
bool: true,
11-
file: await toFile(Buffer.from('some-content')),
12-
blob: new Blob(['Some content'], { type: 'text/plain' }),
6+
await multipartFormRequestOptions(
7+
{
8+
body: {
9+
foo: 'foo',
10+
string: 1,
11+
bool: true,
12+
file: await toFile(Buffer.from('some-content')),
13+
blob: new Blob(['Some content'], { type: 'text/plain' }),
14+
},
1315
},
14-
});
16+
fetch,
17+
);
1518
});
1619

1720
test('null', async () => {
1821
await expect(() =>
19-
multipartFormRequestOptions({
20-
body: {
21-
null: null,
22+
multipartFormRequestOptions(
23+
{
24+
body: {
25+
null: null,
26+
},
2227
},
23-
}),
28+
fetch,
29+
),
2430
).rejects.toThrow(TypeError);
2531
});
2632

2733
test('undefined is stripped', async () => {
28-
const form = await createForm({
29-
foo: undefined,
30-
bar: 'baz',
31-
});
34+
const form = await createForm(
35+
{
36+
foo: undefined,
37+
bar: 'baz',
38+
},
39+
fetch,
40+
);
3241
expect(form.has('foo')).toBe(false);
3342
expect(form.get('bar')).toBe('baz');
3443
});
3544

3645
test('nested undefined property is stripped', async () => {
37-
const form = await createForm({
38-
bar: {
39-
baz: undefined,
46+
const form = await createForm(
47+
{
48+
bar: {
49+
baz: undefined,
50+
},
4051
},
41-
});
52+
fetch,
53+
);
4254
expect(Array.from(form.entries())).toEqual([]);
4355

44-
const form2 = await createForm({
45-
bar: {
46-
foo: 'string',
47-
baz: undefined,
56+
const form2 = await createForm(
57+
{
58+
bar: {
59+
foo: 'string',
60+
baz: undefined,
61+
},
4862
},
49-
});
63+
fetch,
64+
);
5065
expect(Array.from(form2.entries())).toEqual([['bar[foo]', 'string']]);
5166
});
5267

5368
test('nested undefined array item is stripped', async () => {
54-
const form = await createForm({
55-
bar: [undefined, undefined],
56-
});
69+
const form = await createForm(
70+
{
71+
bar: [undefined, undefined],
72+
},
73+
fetch,
74+
);
5775
expect(Array.from(form.entries())).toEqual([]);
5876

59-
const form2 = await createForm({
60-
bar: [undefined, 'foo'],
61-
});
77+
const form2 = await createForm(
78+
{
79+
bar: [undefined, 'foo'],
80+
},
81+
fetch,
82+
);
6283
expect(Array.from(form2.entries())).toEqual([['bar[]', 'foo']]);
6384
});
6485
});

0 commit comments

Comments
 (0)