Skip to content

Commit 2f2f7ba

Browse files
V3 backport[#9048]: Validate input file for Vectorize inserts (#9093)
* fix: Validate input file for Vectorize inserts * VS-398: Throw user errors instead of logging error and returning early for Vectorize commands --------- Co-authored-by: Garvit Gupta <garvit@cloudflare.com>
1 parent b456c87 commit 2f2f7ba

10 files changed

Lines changed: 89 additions & 54 deletions

File tree

.changeset/stupid-paws-slide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: Validate input file for Vectorize inserts

packages/wrangler/src/__tests__/vectorize/vectorize.test.ts

Lines changed: 23 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -339,13 +339,9 @@ describe("vectorize commands", () => {
339339

340340
await expect(
341341
runWrangler("vectorize create test-index --dimensions=1536")
342-
).resolves.toBeUndefined();
343-
344-
expect(std.err).toMatchInlineSnapshot(`
345-
"X [ERROR] You must provide both dimensions and a metric, or a known model preset when creating an index.
346-
347-
"
348-
`);
342+
).rejects.toThrowErrorMatchingInlineSnapshot(
343+
`[Error: 🚨 You must provide both dimensions and a metric, or a known model preset when creating an index.]`
344+
);
349345
});
350346

351347
it("should handle listing vectorize V1 indexes", async () => {
@@ -501,13 +497,12 @@ describe("vectorize commands", () => {
501497

502498
it("should log error when getByIds does not receive ids", async () => {
503499
mockVectorizeV2Request();
504-
await runWrangler("vectorize get-vectors test-index --ids");
505-
506-
expect(std.err).toMatchInlineSnapshot(`
507-
"X [ERROR] 🚨 Please provide valid vector identifiers.
508500

509-
"
510-
`);
501+
await expect(
502+
runWrangler("vectorize get-vectors test-index --ids")
503+
).rejects.toThrowErrorMatchingInlineSnapshot(
504+
`[Error: 🚨 Please provide valid vector identifiers.]`
505+
);
511506
});
512507

513508
it("should handle a deleteByIds on a vectorize index", async () => {
@@ -521,13 +516,12 @@ describe("vectorize commands", () => {
521516

522517
it("should log error when deleteByIds does not receive ids", async () => {
523518
mockVectorizeV2Request();
524-
await runWrangler("vectorize delete-vectors test-index --ids");
525-
526-
expect(std.err).toMatchInlineSnapshot(`
527-
"X [ERROR] 🚨 Please provide valid vector identifiers for deletion.
528519

529-
"
530-
`);
520+
await expect(
521+
runWrangler("vectorize delete-vectors test-index --ids")
522+
).rejects.toThrowErrorMatchingInlineSnapshot(
523+
`[Error: 🚨 Please provide valid vector identifiers for deletion.]`
524+
);
531525
});
532526

533527
it("should handle a query on a vectorize index", async () => {
@@ -590,30 +584,22 @@ describe("vectorize commands", () => {
590584

591585
it("should fail query when neither vector nor vector-id is provided", async () => {
592586
mockVectorizeV2RequestError();
593-
await runWrangler(
594-
"vectorize query test-index --top-k=2 --return-values=true"
587+
await expect(
588+
runWrangler("vectorize query test-index --top-k=2 --return-values=true")
589+
).rejects.toThrowErrorMatchingInlineSnapshot(
590+
`[Error: 🚨 Either vector or vector-id parameter must be provided, but not both.]`
595591
);
596-
expect(std.out).toMatchInlineSnapshot(`""`);
597-
598-
expect(std.err).toMatchInlineSnapshot(`
599-
"X [ERROR] 🚨 Either vector or vector-id parameter must be provided, but not both.
600-
601-
"
602-
`);
603592
});
604593

605594
it("should fail query when both vector and vector-id are provided", async () => {
606595
mockVectorizeV2RequestError();
607-
await runWrangler(
608-
"vectorize query test-index --vector 1 2 3 '4' --vector-id some-vector-id"
596+
await expect(
597+
runWrangler(
598+
"vectorize query test-index --vector 1 2 3 '4' --vector-id some-vector-id"
599+
)
600+
).rejects.toThrowErrorMatchingInlineSnapshot(
601+
`[Error: 🚨 Either vector or vector-id parameter must be provided, but not both.]`
609602
);
610-
expect(std.out).toMatchInlineSnapshot(`""`);
611-
612-
expect(std.err).toMatchInlineSnapshot(`
613-
"X [ERROR] 🚨 Either vector or vector-id parameter must be provided, but not both.
614-
615-
"
616-
`);
617603
});
618604

619605
it("should fail query with invalid return-metadata flag", async () => {

packages/wrangler/src/__tests__/vectorize/vectorize.upsert.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,4 +222,22 @@ describe("dataset upsert", () => {
222222
✅ Successfully enqueued 5 vectors into index 'my-index' for upsertion."
223223
`);
224224
});
225+
226+
it("should reject an invalid file param", async () => {
227+
await expect(
228+
runWrangler("vectorize upsert my-index --file invalid_vectors.ndjson")
229+
).rejects.toThrowErrorMatchingInlineSnapshot(
230+
`[Error: 🚨 Cannot read invalid or empty file: invalid_vectors.ndjson.]`
231+
);
232+
});
233+
234+
it("should reject an empty file param", async () => {
235+
writeFileSync("empty_vectors.ndjson", "");
236+
237+
await expect(
238+
runWrangler("vectorize upsert my-index --file empty_vectors.ndjson")
239+
).rejects.toThrowErrorMatchingInlineSnapshot(
240+
`[Error: 🚨 Cannot read invalid or empty file: empty_vectors.ndjson.]`
241+
);
242+
});
225243
});

packages/wrangler/src/vectorize/common.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { access, constants, stat } from "node:fs/promises";
12
import { logger } from "../logger";
23
import type { Interface as RLInterface } from "node:readline";
34

@@ -18,6 +19,20 @@ export const VECTORIZE_MAX_BATCH_SIZE = 5_000;
1819
export const VECTORIZE_UPSERT_BATCH_SIZE = VECTORIZE_V1_MAX_BATCH_SIZE;
1920
export const VECTORIZE_MAX_UPSERT_VECTOR_RECORDS = 100_000;
2021

22+
// Helper function to carry out initial validations to a file supplied for
23+
// Vectorize upserts/inserts. This function returns true only if the file exists,
24+
// can be read, and is non-empty.
25+
export async function isValidFile(path: string): Promise<boolean> {
26+
try {
27+
await access(path, constants.R_OK);
28+
29+
const fileStat = await stat(path);
30+
return fileStat.isFile() && fileStat.size > 0;
31+
} catch (err) {
32+
return false;
33+
}
34+
}
35+
2136
// helper method that reads an ndjson file line by line in batches. not this doesn't
2237
// actually do any parsing - that will be handled on the backend
2338
// https://nodejs.org/docs/latest-v16.x/api/readline.html#rlsymbolasynciterator

packages/wrangler/src/vectorize/create.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { configFileName, formatConfigSnippet, readConfig } from "../config";
2+
import { UserError } from "../errors";
23
import { logger } from "../logger";
34
import { createIndex } from "./client";
45
import { deprecatedV1DefaultFlag, vectorizeGABanner } from "./common";
@@ -83,10 +84,9 @@ export async function handler(
8384
dimensions: args.dimensions,
8485
};
8586
} else {
86-
logger.error(
87-
"You must provide both dimensions and a metric, or a known model preset when creating an index."
87+
throw new UserError(
88+
"🚨 You must provide both dimensions and a metric, or a known model preset when creating an index."
8889
);
89-
return;
9090
}
9191

9292
if (args.deprecatedV1) {

packages/wrangler/src/vectorize/deleteByIds.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { readConfig } from "../config";
2+
import { UserError } from "../errors";
23
import { logger } from "../logger";
34
import { deleteByIds } from "./client";
45
import { vectorizeGABanner } from "./common";
@@ -33,8 +34,9 @@ export async function handler(
3334
const config = readConfig(args);
3435

3536
if (args.ids.length === 0) {
36-
logger.error("🚨 Please provide valid vector identifiers for deletion.");
37-
return;
37+
throw new UserError(
38+
"🚨 Please provide valid vector identifiers for deletion."
39+
);
3840
}
3941

4042
logger.log(`📋 Deleting vectors...`);

packages/wrangler/src/vectorize/getByIds.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { readConfig } from "../config";
2+
import { UserError } from "../errors";
23
import { logger } from "../logger";
34
import { getByIds } from "./client";
45
import { vectorizeGABanner } from "./common";
@@ -33,8 +34,7 @@ export async function handler(
3334
const config = readConfig(args);
3435

3536
if (args.ids.length === 0) {
36-
logger.error("🚨 Please provide valid vector identifiers.");
37-
return;
37+
throw new UserError("🚨 Please provide valid vector identifiers.");
3838
}
3939

4040
logger.log(`📋 Fetching vectors...`);

packages/wrangler/src/vectorize/insert.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ import { createReadStream } from "node:fs";
22
import { createInterface } from "node:readline";
33
import { File, FormData } from "undici";
44
import { readConfig } from "../config";
5+
import { UserError } from "../errors";
56
import { logger } from "../logger";
67
import { insertIntoIndex, insertIntoIndexV1 } from "./client";
78
import {
89
deprecatedV1DefaultFlag,
910
getBatchFromFile,
11+
isValidFile,
1012
VECTORIZE_MAX_BATCH_SIZE,
1113
VECTORIZE_MAX_UPSERT_VECTOR_RECORDS,
1214
VECTORIZE_UPSERT_BATCH_SIZE,
@@ -56,25 +58,27 @@ export function options(yargs: CommonYargsArgv) {
5658
export async function handler(
5759
args: StrictYargsOptionsToInterface<typeof options>
5860
) {
61+
if (!(await isValidFile(args.file))) {
62+
throw new UserError(`🚨 Cannot read invalid or empty file: ${args.file}.`);
63+
}
64+
5965
const config = readConfig(args);
6066
const rl = createInterface({ input: createReadStream(args.file) });
6167

6268
if (
6369
args.deprecatedV1 &&
6470
Number(args.batchSize) > VECTORIZE_V1_MAX_BATCH_SIZE
6571
) {
66-
logger.error(
72+
throw new UserError(
6773
`🚨 Vectorize currently limits upload batches to ${VECTORIZE_V1_MAX_BATCH_SIZE} records at a time.`
6874
);
69-
return;
7075
} else if (
7176
!args.deprecatedV1 &&
7277
Number(args.batchSize) > VECTORIZE_MAX_BATCH_SIZE
7378
) {
74-
logger.error(
75-
`🚨 The global rate limit for the Cloudflare API is 1200 requests per five minutes. Vectorize V2 indexes currently limit upload batches to ${VECTORIZE_MAX_BATCH_SIZE} records at a time to stay within the service limits`
79+
throw new UserError(
80+
`🚨 The global rate limit for the Cloudflare API is 1200 requests per five minutes. Vectorize V2 indexes currently limit upload batches to ${VECTORIZE_MAX_BATCH_SIZE} records at a time to stay within the service limits.`
7681
);
77-
return;
7882
}
7983

8084
let vectorInsertCount = 0;

packages/wrangler/src/vectorize/query.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { readConfig } from "../config";
2+
import { UserError } from "../errors";
23
import { logger } from "../logger";
34
import { queryIndexByVector, queryIndexByVectorId } from "./client";
45
import { vectorizeGABanner } from "./common";
@@ -121,10 +122,9 @@ export async function handler(
121122
(args.vector === undefined && args.vectorId === undefined) ||
122123
(args.vector !== undefined && args.vectorId !== undefined)
123124
) {
124-
logger.error(
125+
throw new UserError(
125126
"🚨 Either vector or vector-id parameter must be provided, but not both."
126127
);
127-
return;
128128
}
129129

130130
logger.log(`📋 Searching for relevant vectors...`);

packages/wrangler/src/vectorize/upsert.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ import { createReadStream } from "node:fs";
22
import { createInterface } from "node:readline";
33
import { File, FormData } from "undici";
44
import { readConfig } from "../config";
5+
import { UserError } from "../errors";
56
import { logger } from "../logger";
67
import { upsertIntoIndex } from "./client";
78
import {
89
getBatchFromFile,
10+
isValidFile,
911
VECTORIZE_MAX_BATCH_SIZE,
1012
VECTORIZE_MAX_UPSERT_VECTOR_RECORDS,
1113
vectorizeGABanner,
@@ -47,14 +49,17 @@ export function options(yargs: CommonYargsArgv) {
4749
export async function handler(
4850
args: StrictYargsOptionsToInterface<typeof options>
4951
) {
52+
if (!(await isValidFile(args.file))) {
53+
throw new UserError(`🚨 Cannot read invalid or empty file: ${args.file}.`);
54+
}
55+
5056
const config = readConfig(args);
5157
const rl = createInterface({ input: createReadStream(args.file) });
5258

5359
if (Number(args.batchSize) > VECTORIZE_MAX_BATCH_SIZE) {
54-
logger.error(
55-
`🚨 The global rate limit for the Cloudflare API is 1200 requests per five minutes. Vectorize indexes currently limit upload batches to ${VECTORIZE_MAX_BATCH_SIZE} records at a time to stay within the service limits`
60+
throw new UserError(
61+
`🚨 The global rate limit for the Cloudflare API is 1200 requests per five minutes. Vectorize indexes currently limit upload batches to ${VECTORIZE_MAX_BATCH_SIZE} records at a time to stay within the service limits.`
5662
);
57-
return;
5863
}
5964

6065
let vectorUpsertCount = 0;

0 commit comments

Comments
 (0)