fix(client): resumeUpload exits on an expired session instead of uploading into it (#7870) (#8267)

`storage.resumeUpload` polls progress before sending anything, but destructured
only `totalChunks` / `uploadedChunks` and discarded `status`. Since #7667 a
session past its own `expires_at` is durably stamped `expired` and reported by
that poll, so resume walked into the chunk loop and learned the session was dead
from the 410 `UPLOAD_SESSION_EXPIRED` its first chunk PUT returned -- an honest
failure that spent a whole chunk upload to rediscover what it already held.

`expired` now short-circuits before the file is read, throwing an Error carrying
`code: 'UPLOAD_SESSION_EXPIRED'` and `httpStatus: 410` -- the registered code and
status the server answers this same condition with, so a caller's existing
branch keeps matching. Compared with `=== 'expired'` exactly: every other
declared status resumes as before, and an absent `status` cannot misfire it.


Claude-Session: https://claude.ai/code/session_016pY4Xb2iDecfDtT3CWoiTW

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/resume-upload-expired-short-circuit.md

@@ -0,0 +1,26 @@
+---
+'@objectstack/client': patch
+---
+
+client: `storage.resumeUpload` exits on an expired session instead of uploading a chunk into it
+
+`resumeUpload` polls `GET .../upload/chunked/:uploadId/progress` before it sends
+anything, but read only `totalChunks` / `uploadedChunks` off the response and
+discarded `status`. Since #7667 a session past its own `expires_at` is durably
+stamped `expired` and reported as such by that very poll — so a client resuming
+a dead session learned nothing from the response it already had, uploaded a full
+chunk, and discovered the expiry from the 410 `UPLOAD_SESSION_EXPIRED` the chunk
+`PUT` came back with. Correct, but it spent an upload to rediscover something it
+had been told.
+
+The poll's `status` is now read: `expired` short-circuits before the file is
+read or a single byte leaves, throwing an `Error` carrying
+`code: 'UPLOAD_SESSION_EXPIRED'` and `httpStatus: 410` — deliberately the same
+registered code and status the server answers a chunk `PUT` against that session
+with, plus `details: { uploadId, expiresAt }`. A caller already branching on
+`err.code === 'UPLOAD_SESSION_EXPIRED'` keeps matching; the difference is only
+how early it fires, and that the bytes stay home.
+
+The guard compares against `'expired'` exactly, so every other declared status
+(`in_progress`, `completing`, `completed`, `failed`) resumes as before, and a
+server or fixture that omits `status` is unaffected.

packages/client/src/index.ts

@@ -2943,6 +2943,11 @@ export class ObjectStackClient {
     /**
      * Resume an interrupted chunked upload.
      * Fetches current progress, then uploads remaining chunks and completes.
+     *
+     * Throws before uploading anything when the progress poll reports the
+     * session as `expired` — an `Error` carrying `code`
+     * `'UPLOAD_SESSION_EXPIRED'` and `httpStatus` 410, the same pair the server
+     * answers a chunk PUT against a dead session with (#7870).
      */
     resumeUpload: async (uploadId: string, file: Blob | ArrayBuffer, chunkSize: number, resumeToken: string): Promise<CompleteChunkedUploadResponse> => {
         const route = this.getRoute('storage');
@@ -2951,7 +2956,42 @@ export class ObjectStackClient {
         const progressRes = await this.fetch(`${this.baseUrl}${route}/upload/chunked/${uploadId}/progress`);
         const progress = await progressRes.json() as UploadProgress;
 
-        const { totalChunks, uploadedChunks } = progress.data;
+        const { totalChunks, uploadedChunks, status, expiresAt } = progress.data;
+
+        // [#7870] A session past its own `expires_at` is durably stamped
+        // `expired` by the server (#7667), and THIS poll is where it says so —
+        // `status` is a declared member of `UploadProgressSchema`, populated on
+        // every progress read. Before this check the value was fetched and
+        // dropped: resume walked straight into the chunk loop and learned the
+        // session was dead from the 410 `UPLOAD_SESSION_EXPIRED` its first
+        // chunk PUT came back with. Honest, but it spent a whole chunk upload
+        // to rediscover something the response already in hand had told it.
+        //
+        // The code and status deliberately MIRROR that 410 rather than naming a
+        // new condition: `UPLOAD_SESSION_EXPIRED` is the registered code the
+        // server answers this exact case with (error-code-ledger.zod.ts), so a
+        // caller's existing `err.code === 'UPLOAD_SESSION_EXPIRED'` branch
+        // fires identically whether the expiry was caught here or by the
+        // server. Same error shape as the `fetch` wrapper builds for a real
+        // non-2xx (message + `code`/`httpStatus`/`details`) — this is an
+        // earlier detection of one condition, not a second one.
+        //
+        // Compared with `=== 'expired'`, never truthiness: the other declared
+        // statuses (`in_progress`, `completing`, `completed`, `failed`) all
+        // proceed exactly as before — `failed` and wider status handling are
+        // deliberately out of scope — and an absent `status` from a server or
+        // fixture that omits it cannot misfire the short-circuit.
+        if (status === 'expired') {
+            const expiredError = new Error(
+                `Upload session ${uploadId} expired${expiresAt ? ` at ${expiresAt}` : ''}`
+                + '; start a new chunked upload',
+            ) as Error & { code: string; httpStatus: number; details: Record<string, any> };
+            expiredError.code = 'UPLOAD_SESSION_EXPIRED';
+            expiredError.httpStatus = 410;
+            expiredError.details = { uploadId, expiresAt };
+            throw expiredError;
+        }
+
         const parts: Array<{ chunkIndex: number; eTag: string }> = [];
 
         // 2. Upload remaining chunks

packages/client/src/storage-wire-dialect.test.ts

@@ -136,3 +136,113 @@ describe('the enveloped storage responses match their declared return types (#36
     expect(res.data.eTag).toBe('"abc"');
   });
 });
+
+/**
+ * `resumeUpload` short-circuits on the expiry the progress poll already told it
+ * about (#7870).
+ *
+ * Since #7667 a chunked session past its own `expires_at` is durably stamped
+ * `expired`, `GET .../progress` reports that status, and a chunk PUT against it
+ * answers 410 `UPLOAD_SESSION_EXPIRED`. `resumeUpload` polls progress FIRST but
+ * read only the chunk counters off it, so it uploaded a full chunk into a
+ * session the poll had already declared dead and learned that from the 410.
+ *
+ * What these pin is the pair a caller actually branches on — `code` AND
+ * `httpStatus` — not merely that something threw: the point of the fix is that
+ * the early exit is INDISTINGUISHABLE from the server's own refusal, so a
+ * `catch` written against the 410 keeps matching. Asserting only "it throws"
+ * would stay green if the short-circuit raised a bare `Error`, which is the one
+ * outcome that would break every such caller.
+ */
+describe('storage.resumeUpload exits on an expired session before uploading (#7870)', () => {
+  /** The progress body the server sends for a session past its deadline. */
+  const expiredProgress = {
+    success: true,
+    data: {
+      uploadId: 'up1',
+      fileId: 'f1',
+      filename: 'big.bin',
+      totalSize: 16,
+      uploadedSize: 8,
+      totalChunks: 2,
+      uploadedChunks: 1,
+      percentComplete: 50,
+      status: 'expired',
+      startedAt: '2026-01-01T00:00:00.000Z',
+      expiresAt: '2026-01-01T01:00:00.000Z',
+    },
+  };
+
+  it('throws the registered code and status the server answers a dead session with', async () => {
+    const { client } = clientReturning(expiredProgress);
+
+    // `.rejects.toThrow()` alone cannot see the difference between this and a
+    // bare Error, so the envelope is asserted off the caught object.
+    const err = await client.storage
+      .resumeUpload('up1', new ArrayBuffer(16), 8, 'rtok')
+      .then(() => null, (e: any) => e);
+
+    expect(err).toBeInstanceOf(Error);
+    expect(err.code).toBe('UPLOAD_SESSION_EXPIRED');
+    expect(err.httpStatus).toBe(410);
+    // The expiry instant is what tells a caller/operator WHICH deadline passed,
+    // so it survives into both the human message and `details`.
+    expect(err.message).toContain('2026-01-01T01:00:00.000Z');
+    expect(err.details).toMatchObject({ uploadId: 'up1', expiresAt: '2026-01-01T01:00:00.000Z' });
+  });
+
+  it('sends no chunk PUT and no complete — the poll is the only request made', async () => {
+    // The whole point of the card: the bytes never leave. If this regresses,
+    // the throw above would still pass while the client had already spent a
+    // chunk upload against a dead session.
+    const { client, fetchMock } = clientReturning(expiredProgress);
+
+    await client.storage.resumeUpload('up1', new ArrayBuffer(16), 8, 'rtok').catch(() => {});
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock.mock.calls[0][0]).toContain('/upload/chunked/up1/progress');
+  });
+
+  it('still resumes normally when the session is live', async () => {
+    // The reverse direction: `=== 'expired'` must not swallow the happy path.
+    // One mock, routed by URL — resume makes three hops (progress, chunk PUT,
+    // complete) and they answer different bodies.
+    const fetchMock = vi.fn(async (url: string) => {
+      const body = url.includes('/progress')
+        ? { success: true, data: { ...expiredProgress.data, status: 'in_progress' } }
+        : url.includes('/complete')
+          ? { success: true, data: { fileId: 'f1', size: 16 } }
+          : { success: true, data: { chunkIndex: 1, eTag: '"e2"', bytesReceived: 8 } };
+      return { ok: true, status: 200, statusText: 'OK', json: async () => body, headers: new Headers() };
+    });
+    const client = new ObjectStackClient({ baseUrl: 'http://localhost:3000', fetch: fetchMock as any });
+
+    const res = await client.storage.resumeUpload('up1', new ArrayBuffer(16), 8, 'rtok');
+
+    expect(res.success).toBe(true);
+    expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual([
+      expect.stringContaining('/upload/chunked/up1/progress'),
+      expect.stringContaining('/upload/chunked/up1/chunk/1'),
+      expect.stringContaining('/upload/chunked/up1/complete'),
+    ]);
+  });
+
+  it('does not misfire when a server or fixture omits status entirely', async () => {
+    // `status` is declared required, but the client is published separately
+    // from the server it meets and the SDK's own URL-conformance fixture drives
+    // this method with a counters-only body. An absent status must resume, not
+    // abort — which is why the guard compares against 'expired' rather than
+    // testing truthiness or absence.
+    const fetchMock = vi.fn(async (url: string) => {
+      const body = url.includes('/progress')
+        ? { success: true, data: { totalChunks: 1, uploadedChunks: 0 } }
+        : { success: true, data: { chunkIndex: 0, eTag: '"e1"', bytesReceived: 8 } };
+      return { ok: true, status: 200, statusText: 'OK', json: async () => body, headers: new Headers() };
+    });
+    const client = new ObjectStackClient({ baseUrl: 'http://localhost:3000', fetch: fetchMock as any });
+
+    await expect(
+      client.storage.resumeUpload('up1', new ArrayBuffer(8), 8, 'rtok'),
+    ).resolves.toBeDefined();
+  });
+});