feat(kv): return ok bool from atomic commit (#18873)

3 files changed, 52 insertions, 36 deletions

diff --git a/cli/tests/unit/kv_test.ts b/cli/tests/unit/kv_test.ts
index 0af8f338d..0dc1690aa 100644
--- a/cli/tests/unit/kv_test.ts
+++ b/cli/tests/unit/kv_test.ts
@@ -66,6 +66,7 @@ dbTest("basic read-write-delete and versionstamps", async (db) => {
   assertEquals(result1.versionstamp, null);
 
   const setRes = await db.set(["a"], "b");
+  assert(setRes.ok);
   assertEquals(setRes.versionstamp, "00000000000000010000");
   const result2 = await db.get(["a"]);
   assertEquals(result2.key, ["a"]);
@@ -183,7 +184,7 @@ dbTest("compare and mutate", async (db) => {
     .check({ key: ["t"], versionstamp: currentValue.versionstamp })
     .set(currentValue.key, "2")
     .commit();
-  assert(res);
+  assert(res.ok);
   assertEquals(res.versionstamp, "00000000000000020000");
 
   const newValue = await db.get(["t"]);
@@ -194,7 +195,7 @@ dbTest("compare and mutate", async (db) => {
     .check({ key: ["t"], versionstamp: currentValue.versionstamp })
     .set(currentValue.key, "3")
     .commit();
-  assertEquals(res, null);
+  assert(!res.ok);
 
   const newValue2 = await db.get(["t"]);
   assertEquals(newValue2.versionstamp, "00000000000000020000");
@@ -206,7 +207,7 @@ dbTest("compare and mutate not exists", async (db) => {
     .check({ key: ["t"], versionstamp: null })
     .set(["t"], "1")
     .commit();
-  assert(res);
+  assert(res.ok);
 
   const newValue = await db.get(["t"]);
   assertEquals(newValue.versionstamp, "00000000000000010000");
@@ -216,7 +217,7 @@ dbTest("compare and mutate not exists", async (db) => {
     .check({ key: ["t"], versionstamp: null })
     .set(["t"], "2")
     .commit();
-  assertEquals(res, null);
+  assert(!res.ok);
 });
 
 dbTest("atomic mutation helper (sum)", async (db) => {
@@ -264,7 +265,7 @@ dbTest("compare multiple and mutate", async (db) => {
     .set(currentValue1.key, "3")
     .set(currentValue2.key, "4")
     .commit();
-  assert(res);
+  assert(res.ok);
 
   const newValue1 = await db.get(["t1"]);
   assertEquals(newValue1.versionstamp, "00000000000000030000");
@@ -280,7 +281,7 @@ dbTest("compare multiple and mutate", async (db) => {
     .set(newValue1.key, "5")
     .set(newValue2.key, "6")
     .commit();
-  assertEquals(res2, null);
+  assert(!res2.ok);
 
   const newValue3 = await db.get(["t1"]);
   assertEquals(newValue3.versionstamp, "00000000000000030000");
@@ -296,7 +297,7 @@ dbTest("atomic mutation ordering (set before delete)", async (db) => {
     .set(["a"], "2")
     .delete(["a"])
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, null);
 });
@@ -307,7 +308,7 @@ dbTest("atomic mutation ordering (delete before set)", async (db) => {
     .delete(["a"])
     .set(["a"], "2")
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, "2");
 });
@@ -316,7 +317,7 @@ dbTest("atomic mutation type=set", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: "1", type: "set" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, "1");
 });
@@ -326,7 +327,7 @@ dbTest("atomic mutation type=set overwrite", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: "2", type: "set" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, "2");
 });
@@ -336,7 +337,7 @@ dbTest("atomic mutation type=delete", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], type: "delete" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, null);
 });
@@ -345,7 +346,7 @@ dbTest("atomic mutation type=delete no exists", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], type: "delete" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, null);
 });
@@ -355,7 +356,7 @@ dbTest("atomic mutation type=sum", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "sum" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, new Deno.KvU64(11n));
 });
@@ -364,7 +365,7 @@ dbTest("atomic mutation type=sum no exists", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "sum" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assert(result.value);
   assertEquals(result.value, new Deno.KvU64(1n));
@@ -375,7 +376,7 @@ dbTest("atomic mutation type=sum wrap around", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(10n), type: "sum" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, new Deno.KvU64(9n));
 
@@ -423,7 +424,7 @@ dbTest("atomic mutation type=min", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(5n), type: "min" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, new Deno.KvU64(5n));
 
@@ -439,7 +440,7 @@ dbTest("atomic mutation type=min no exists", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "min" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assert(result.value);
   assertEquals(result.value, new Deno.KvU64(1n));
@@ -477,7 +478,7 @@ dbTest("atomic mutation type=max", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(5n), type: "max" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assertEquals(result.value, new Deno.KvU64(10n));
 
@@ -493,7 +494,7 @@ dbTest("atomic mutation type=max no exists", async (db) => {
   const res = await db.atomic()
     .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "max" })
     .commit();
-  assert(res);
+  assert(res.ok);
   const result = await db.get(["a"]);
   assert(result.value);
   assertEquals(result.value, new Deno.KvU64(1n));
diff --git a/cli/tsc/dts/lib.deno.unstable.d.ts b/cli/tsc/dts/lib.deno.unstable.d.ts
index f169e0254..8613da2ab 100644
--- a/cli/tsc/dts/lib.deno.unstable.d.ts
+++ b/cli/tsc/dts/lib.deno.unstable.d.ts
@@ -1545,6 +1545,10 @@ declare namespace Deno {
    * relative significance of the types can be found in documentation for the
    * {@linkcode Deno.KvKeyPart} type.
    *
+   * Keys have a maximum size of 2048 bytes serialized. If the size of the key
+   * exceeds this limit, an error will be thrown on the operation that this key
+   * was passed to.
+   *
    * @category KV
    */
   export type KvKey = readonly KvKeyPart[];
@@ -1758,10 +1762,16 @@ declare namespace Deno {
 
   /** @category KV */
   export interface KvCommitResult {
+    ok: true;
     /** The versionstamp of the value committed to KV. */
     versionstamp: string;
   }
 
+  /** @category KV */
+  export interface KvCommitError {
+    ok: false;
+  }
+
   /** **UNSTABLE**: New API, yet to be vetted.
    *
    * A check to perform as part of a {@linkcode Deno.AtomicOperation}. The check
@@ -1803,11 +1813,13 @@ declare namespace Deno {
    *
    * The `commit` method of an atomic operation returns a value indicating
    * whether checks passed and mutations were performed. If the operation failed
-   * because of a failed check, the return value will be `null`. If the
+   * because of a failed check, the return value will be a
+   * {@linkcode Deno.KvCommitError} with an `ok: false` property. If the
    * operation failed for any other reason (storage error, invalid value, etc.),
    * an exception will be thrown. If the operation succeeded, the return value
-   * will be a {@linkcode Deno.KvCommitResult} object containing the
-   * versionstamp of the value committed to KV.
+   * will be a {@linkcode Deno.KvCommitResult} object with a `ok: true` property
+   * and the versionstamp of the value committed to KV.
+
    *
    * @category KV
    */
@@ -1857,17 +1869,19 @@ declare namespace Deno {
     /**
      * Commit the operation to the KV store. Returns a value indicating whether
      * checks passed and mutations were performed. If the operation failed
-     * because of a failed check, the return value will be `null`. If the
-     * operation failed for any other reason (storage error, invalid value,
-     * etc.), an exception will be thrown. If the operation succeeded, the
-     * return value will be a {@linkcode Deno.KvCommitResult} object containing
-     * the versionstamp of the value committed to KV.
+     * because of a failed check, the return value will be a {@linkcode
+     * Deno.KvCommitError} with an `ok: false` property. If the operation failed
+     * for any other reason (storage error, invalid value, etc.), an exception
+     * will be thrown. If the operation succeeded, the return value will be a
+     * {@linkcode Deno.KvCommitResult} object with a `ok: true` property and the
+     * versionstamp of the value committed to KV.
      *
-     * If the commit returns `null`, one may create a new atomic operation with
-     * updated checks and mutations and attempt to commit it again. See the note
-     * on optimistic locking in the documentation for {@linkcode Deno.AtomicOperation}.
+     * If the commit returns `ok: false`, one may create a new atomic operation
+     * with updated checks and mutations and attempt to commit it again. See the
+     * note on optimistic locking in the documentation for
+     * {@linkcode Deno.AtomicOperation}.
      */
-    commit(): Promise<KvCommitResult | null>;
+    commit(): Promise<KvCommitResult | KvCommitError>;
   }
 
   /** **UNSTABLE**: New API, yet to be vetted.
@@ -1901,7 +1915,8 @@ declare namespace Deno {
    * maximum length of 64 KiB after serialization. Serialization of both keys
    * and values is somewhat opaque, but one can usually assume that the
    * serialization of any value is about the same length as the resulting string
-   * of a JSON serialization of that same value.
+   * of a JSON serialization of that same value. If theses limits are exceeded,
+   * an exception will be thrown.
    *
    * @category KV
    */
diff --git a/ext/kv/01_db.ts b/ext/kv/01_db.ts
index da29a0952..0dd6ba83a 100644
--- a/ext/kv/01_db.ts
+++ b/ext/kv/01_db.ts
@@ -116,7 +116,7 @@ class Kv {
       [],
     );
     if (versionstamp === null) throw new TypeError("Failed to set value");
-    return { versionstamp };
+    return { ok: true, versionstamp };
   }
 
   async delete(key: Deno.KvKey) {
@@ -266,7 +266,7 @@ class AtomicOperation {
     return this;
   }
 
-  async commit(): Promise<Deno.KvCommitResult | null> {
+  async commit(): Promise<Deno.KvCommitResult | Deno.KvCommitError> {
     const versionstamp = await core.opAsync(
       "op_kv_atomic_write",
       this.#rid,
@@ -274,8 +274,8 @@ class AtomicOperation {
       this.#mutations,
       [], // TODO(@losfair): enqueue
     );
-    if (versionstamp === null) return null;
-    return { versionstamp };
+    if (versionstamp === null) return { ok: false };
+    return { ok: true, versionstamp };
   }
 
   then() {