Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- summary: |
Fix `TypeError: Cannot use 'in' operator to search for 'path' in undefined`
crash when `null` or `undefined` is passed to a file upload parameter at
runtime. The error is now a clear `TypeError` describing the expected types
instead of a cryptic `in` operator failure.
type: fix
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ async function getFileWithMetadata(
file: Uploadable,
{ noSniffFileSize }: { noSniffFileSize?: boolean } = {},
): Promise<Uploadable.WithMetadata> {
if (file == null) {
throw new Error(
`Expected file to be a Blob, Buffer, ReadableStream, or an object with a "path" or "data" property, but received ${file === null ? "null" : "undefined"}.`,
);
}
if (isFileLike(file)) {
return getFileWithMetadata(
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export class Node18FormData implements CrossPlatformFormData {
}

public async appendFile(key: string, value: Uploadable): Promise<void> {
if (value == null) {
console.warn(`File upload for "${key}" received ${value === null ? "null" : "undefined"}, skipping.`);
return;
}
Comment on lines +82 to +86

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should throw here. appendFile should not be allowed to be called with null.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warn+skip behavior here was specifically requested to match the old 3.12.x SDK tolerance — the customer's optional file field passes undefined through the generated code into appendFile, and they expect it not to crash.

If the intent is to throw here instead, the fix would need to move upstream: the generated code that calls appendFile would need to guard optional file fields with if (value != null) before calling appendFile. Without that generated-code change, throwing here would reproduce the original crash (just with a clearer error message).

@cadesark — could you weigh in on the approach? Should we:

  1. Keep warn+skip in appendFile (current PR — tolerant core utility), or
  2. Throw in appendFile but also fix the generated caller code to not call appendFile with null for optional fields?

Option 2 is the cleaner design but requires changes to the TypeScript SDK generator's request body serialization.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — appendFile now throws an Error instead of warn+skip. The generated code already guards optional file fields with if (request.maybe_file != null) before calling appendFile, so this is safe.

const { data, filename } = await toMultipartDataPart(value);

if (data instanceof Blob) {
Expand Down Expand Up @@ -140,6 +144,10 @@ export class Node16FormData implements CrossPlatformFormData {
}

public async appendFile(key: string, value: Uploadable): Promise<void> {
if (value == null) {
console.warn(`File upload for "${key}" received ${value === null ? "null" : "undefined"}, skipping.`);
return;
}
const { data, filename } = await toMultipartDataPart(value);

let bufferedValue;
Expand Down Expand Up @@ -181,6 +189,10 @@ export class WebFormData implements CrossPlatformFormData {
}

public async appendFile(key: string, value: Uploadable): Promise<void> {
if (value == null) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should throw here. appendFile should not be allowed to be called with null.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above — this is part of the same decision. If we throw in appendFile, the null guard for optional file fields needs to live in the generated caller code instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — now throws.

console.warn(`File upload for "${key}" received ${value === null ? "null" : "undefined"}, skipping.`);
return;
}
const { data, filename, contentType } = await toMultipartDataPart(value);

if (data instanceof Blob) {
Expand Down Expand Up @@ -221,6 +233,10 @@ export class FormDataWrapper {
}

public async appendFile(key: string, value: Uploadable): Promise<void> {
if (value == null) {
console.warn(`File upload for "${key}" received ${value === null ? "null" : "undefined"}, skipping.`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should throw here. appendFile should not be allowed to be called with null.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above — this is part of the same decision. If we throw in appendFile, the null guard for optional file fields needs to live in the generated caller code instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — now throws.

return;
}
const { data, filename, contentType } = await toMultipartDataPart(value);
const blob = await convertToBlob(data, contentType);
if (filename) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,22 @@ describe("toBinaryUploadRequest", () => {
});
});

describe("null and undefined input", () => {
it("should throw Error for undefined in binary upload", async () => {
await expect(toBinaryUploadRequest(undefined as unknown as Uploadable)).rejects.toThrow(Error);
await expect(toBinaryUploadRequest(undefined as unknown as Uploadable)).rejects.toThrow(
/but received undefined/,
);
});

it("should throw Error for null in binary upload", async () => {
await expect(toBinaryUploadRequest(null as unknown as Uploadable)).rejects.toThrow(Error);
await expect(toBinaryUploadRequest(null as unknown as Uploadable)).rejects.toThrow(
/but received null/,
);
});
});

describe("Edge cases", () => {
it("should handle empty headers when no metadata is available", async () => {
const buffer = Buffer.from("");
Expand Down
5 changes: 5 additions & 0 deletions seed/ts-sdk/bytes-upload/src/core/file/file.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions seed/ts-sdk/bytes-upload/tests/unit/file/file.test.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions seed/ts-sdk/enum/no-custom-config/src/core/file/file.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions seed/ts-sdk/enum/no-custom-config/tests/unit/file/file.test.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions seed/ts-sdk/enum/serde/src/core/file/file.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions seed/ts-sdk/enum/serde/tests/unit/file/file.test.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions seed/ts-sdk/exhaustive/bigint/src/core/file/file.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading