From ea2f9967a67db9c936097c71556628d2005626a4 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Fri, 14 Aug 2026 00:26:07 -0700 Subject: [PATCH] fix: over-quota batch uploads surface the storage prompt instead of failing quietly (#3566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: over-quota batch uploads surface the storage prompt instead of failing quietly Partial batch failures now carry each item's code/status, and when every failed item failed the same way the shared code/status is hoisted onto the rejection itself — so an upload that exceeds the storage quota rejects with storage_limit_reached/413 and the SDK's upload handler shows the free-up-space prompt. A partial failure is also no longer misread as the signed-batch endpoint being unavailable. Co-Authored-By: Claude Fable 5 * fix: out-of-storage copies prompt to upgrade the same way uploads do Any fs operation the server refuses with 413 storage_limit_reached now surfaces the upgrade prompt — the check that lived inline in upload's error handler moves to a shared helper wired into the operation scaffold's reject path, so copy/move/mkdir/rename get it too. The desktop's copy/paste suppresses its generic alert for that code, since the SDK dialog already explains the refusal and carries the fix. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- src/docs/src/FS/upload.md | 4 +- src/gui/src/helpers.js | 13 +++- .../modules/FileSystem/operations/scaffold.js | 11 ++- .../operations/storageLimitPrompt.js | 45 +++++++++++ .../operations/storageLimitPrompt.test.js | 58 ++++++++++++++ .../FileSystem/operations/upload/index.js | 16 +--- .../operations/upload/signedBatchUpload.js | 33 ++++++++ .../upload/signedBatchUpload.test.js | 76 +++++++++++++++++++ 8 files changed, 239 insertions(+), 17 deletions(-) create mode 100644 src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.js create mode 100644 src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.test.js create mode 100644 src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.test.js diff --git a/src/docs/src/FS/upload.md b/src/docs/src/FS/upload.md index 9f04b5efd..cbf321db6 100755 --- a/src/docs/src/FS/upload.md +++ b/src/docs/src/FS/upload.md @@ -54,7 +54,9 @@ Returns a `Promise` that resolves to: - A single [`FSItem`](/Objects/fsitem/) object if `items` parameter contains one item - An array of [`FSItem`](/Objects/fsitem/) objects if `items` parameter contains multiple items -If any part of the upload fails, the promise is rejected — it never resolves to a mix of items and errors. The rejection value always carries a `message`, and a `failedItems` array when individual items failed rather than the request as a whole. A partially failed upload is not rolled back: the items that were written stay written. +If any part of the upload fails, the promise is rejected — it never resolves to a mix of items and errors. The rejection value always carries a `message`, and a `failedItems` array when individual items failed rather than the request as a whole. Each entry in `failedItems` carries the `path`, `message`, and — when the server gave one — the `code` and `status` for that item. A partially failed upload is not rolled back: the items that were written stay written. + +When every failed item failed the same way, that `code` and `status` are also set on the rejection value itself, because the cause belongs to the request rather than to any one file. An upload that exceeds the account's storage quota is the common case: it rejects with `code: 'storage_limit_reached'` and `status: 413` however many files were in it. On `nodejs` and `workers`, where the upload goes through an older batch endpoint, the rejection value also carries a stable `code`: diff --git a/src/gui/src/helpers.js b/src/gui/src/helpers.js index 4f64a4fc7..fd02387ef 100644 --- a/src/gui/src/helpers.js +++ b/src/gui/src/helpers.js @@ -1352,7 +1352,10 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element) } } else { - if ( err.message ) { + // An out-of-storage copy already shows the SDK's + // upgrade dialog — a second, generic alert on top of + // it would just bury the actionable one. + if ( err.message && err.code !== 'storage_limit_reached' ) { UIAlert(err.message); } item_with_same_name_already_exists = false; @@ -1488,7 +1491,13 @@ window.copy_items = function (el_items, dest_path) { } } else { - if ( err.message ) { + // An out-of-storage copy already shows the SDK's + // upgrade dialog — a second, generic alert on top of + // it would just bury the actionable one. + if ( err.code === 'storage_limit_reached' ) { + // handled by the SDK prompt + } + else if ( err.message ) { UIAlert(err.message); } else if ( err ) { diff --git a/src/puter-js/src/modules/FileSystem/operations/scaffold.js b/src/puter-js/src/modules/FileSystem/operations/scaffold.js index 07fdb2bb2..9a6ed5b99 100644 --- a/src/puter-js/src/modules/FileSystem/operations/scaffold.js +++ b/src/puter-js/src/modules/FileSystem/operations/scaffold.js @@ -8,6 +8,7 @@ // positional arguments and the request they make. import * as utils from '../../../lib/utils.js'; +import { promptIfStorageLimitError } from './storageLimitPrompt.js'; /** @typedef {import('../index.js').PuterJSFileSystemModule} FileSystemModule */ @@ -151,6 +152,14 @@ export async function fsRequest (spec) { prepareXhr?.(xhr); + // Storage refusals (413 on copy, mkdir, ...) prompt the user to + // upgrade the same way an over-quota upload does — on the reject path + // only, so the prompt fires once however many callbacks are attached. + const rejectWithPrompt = (e) => { + promptIfStorageLimitError(e); + reject(e); + }; + // `transform` has to run before the callbacks so that they and the // promise agree on the value, which rules out `setupXhrEventHandlers`' // own success callback. @@ -163,7 +172,7 @@ export async function fsRequest (spec) { if ( typeof error === 'function' ) error(e); reject(e); } - }, reject); + }, rejectWithPrompt); xhr.send(body === undefined ? undefined : JSON.stringify(body)); }); diff --git a/src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.js b/src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.js new file mode 100644 index 000000000..ca8e51072 --- /dev/null +++ b/src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.js @@ -0,0 +1,45 @@ +// The out-of-storage upgrade prompt, shared by every filesystem operation. +// +// Any operation that allocates bytes can be refused with 413 +// `storage_limit_reached` — uploads, but also copies (a copy duplicates +// bytes) and the rest of the mutation surface. The refusal reaches the app +// as a rejection either way; this is the part that also tells the *user*, +// so an app that swallows the rejection doesn't read as "Puter stopped +// saving my files". Mirrors the credit flow: prompt AND reject, never +// prompt instead of rejecting. + +import { showUsageLimitDialog } from '../../UsageLimitDialog.js'; + +/** + * Whether a filesystem rejection means the account is out of storage. + * Matches every shape the storage refusal arrives in: the structured + * `code` from the API error body, the bare 413 status, and the legacy + * batch endpoint's `NOT_ENOUGH_SPACE`. + * + * @param {unknown} error + * @returns {boolean} + */ +export const isStorageLimitError = (error) => { + if ( !error || typeof error !== 'object' ) return false; + const e = /** @type {{ code?: unknown, status?: unknown }} */ (error); + return e.code === 'storage_limit_reached' + || e.code === 'NOT_ENOUGH_SPACE' + || e.status === 413; +}; + +/** + * Show the upgrade prompt when `error` is a storage refusal: the app's own + * upgrade flow inside an app, the usage-limit dialog everywhere else (which + * dedupes itself, so racing operations can't stack dialogs). The error is + * not consumed — callers still reject with it. + * + * @param {unknown} error + */ +export const promptIfStorageLimitError = (error) => { + if ( !isStorageLimitError(error) ) return; + if ( puter.env === 'app' ) { + puter.ui.requestUpgrade(); + } else { + showUsageLimitDialog('Not enough storage space available.
Please upgrade to continue.'); + } +}; diff --git a/src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.test.js b/src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.test.js new file mode 100644 index 000000000..972ef0ddb --- /dev/null +++ b/src/puter-js/src/modules/FileSystem/operations/storageLimitPrompt.test.js @@ -0,0 +1,58 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { isStorageLimitError, promptIfStorageLimitError } from './storageLimitPrompt.js'; + +// Every filesystem operation routes its rejections through this helper, so an +// over-quota copy or mkdir prompts the user to upgrade exactly like an +// over-quota upload does — instead of only rejecting into an app that may +// swallow the error. +describe('isStorageLimitError', () => { + it.each([ + [{ code: 'storage_limit_reached' }, true], + [{ status: 413 }, true], + [{ code: 'NOT_ENOUGH_SPACE' }, true], + [{ code: 'storage_limit_reached', status: 413 }, true], + [{ code: 'item_with_same_name_exists' }, false], + [{ status: 403 }, false], + [{ message: 'network error' }, false], + ['Storage limit reached', false], + [null, false], + [undefined, false], + ])('%o -> %s', (error, expected) => { + expect(isStorageLimitError(error)).toBe(expected); + }); +}); + +describe('promptIfStorageLimitError', () => { + const origPuter = globalThis.puter; + + beforeEach(() => { + globalThis.puter = { env: 'app', ui: { requestUpgrade: vi.fn() } }; + }); + + afterEach(() => { + globalThis.puter = origPuter; + }); + + it('hands off to the app upgrade flow inside an app', () => { + promptIfStorageLimitError({ code: 'storage_limit_reached' }); + expect(globalThis.puter.ui.requestUpgrade).toHaveBeenCalledTimes(1); + }); + + it('does nothing for a non-storage rejection', () => { + promptIfStorageLimitError({ status: 403 }); + promptIfStorageLimitError(new Error('offline')); + expect(globalThis.puter.ui.requestUpgrade).not.toHaveBeenCalled(); + }); + + it('outside an app it warns rather than throwing without a DOM', () => { + // In node there is no document; showUsageLimitDialog logs instead. + globalThis.puter.env = 'web'; + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + expect(() => + promptIfStorageLimitError({ status: 413 }), + ).not.toThrow(); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); +}); diff --git a/src/puter-js/src/modules/FileSystem/operations/upload/index.js b/src/puter-js/src/modules/FileSystem/operations/upload/index.js index 606cf16da..589ffd17a 100644 --- a/src/puter-js/src/modules/FileSystem/operations/upload/index.js +++ b/src/puter-js/src/modules/FileSystem/operations/upload/index.js @@ -4,8 +4,8 @@ // `/batch` path when signed writes are unavailable). import * as utils from '../../../../lib/utils.js'; -import { showUsageLimitDialog } from '../../../UsageLimitDialog.js'; import getAbsolutePathForApp from '../../utils/getAbsolutePathForApp.js'; +import { promptIfStorageLimitError } from '../storageLimitPrompt.js'; import { SIGNED_BATCH_WRITE_CAPABILITY_KEY, SIGNED_BATCH_SUPPORTED_ENVS, SPACE_CHECK_MIN_BYTES } from './constants.js'; import { normalizeUploadEntries, separateFilesAndDirs } from './entries.js'; import { generateThumbnails } from './thumbnails.js'; @@ -52,18 +52,8 @@ const uploadImpl = async function (items, dirPath, options = {}) { } const error = (e) => { - // Check for storage limit errors and show upgrade dialog - const isStorageError = - e?.code === 'NOT_ENOUGH_SPACE' || - e?.status === 413 || - e?.code === 'storage_limit_reached'; - if ( isStorageError ) { - if ( puter.env === 'app' ) { - puter.ui.requestUpgrade(); - } else { - showUsageLimitDialog('Not enough storage space available.
Please upgrade to continue.'); - } - } + // Out of storage: prompt the user to upgrade, then reject as usual. + promptIfStorageLimitError(e); // if error callback is provided, call it if ( options.error && typeof options.error === 'function' ) diff --git a/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.js b/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.js index 216108908..e76322822 100644 --- a/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.js +++ b/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.js @@ -29,6 +29,10 @@ import { chunkArray } from './entries.js'; export const isSignedBatchWriteUnavailableError = (error) => { if ( !error || typeof error !== 'object' ) return false; if ( error.signedBatchUnavailable === true ) return true; + // A partial failure proves the endpoints exist — the batch got far enough + // to fail per item. It carries the items' shared status, which must not be + // read as the endpoint's own answer. + if ( error.partial === true ) return false; const errorBody = error.body && typeof error.body === 'object' ? error.body : null; @@ -43,6 +47,32 @@ export const isSignedBatchWriteUnavailableError = (error) => { return SIGNED_BATCH_WRITE_UNAVAILABLE_STATUSES.has(error.status); }; +/** + * The `code` / `status` shared by every failed item of a batch, if there is + * one. A rejection they all carry is a property of the request rather than of + * any single file — an over-quota account is the case that matters, since the + * allowance check fails the whole batch when it is completed. Hoisting it onto + * the thrown error is what lets callers keying on `error.code` recognise it + * (the SDK's own upload handler included, which prompts to free up space) + * instead of seeing a generic "operations failed". + * + * Mixed failures report nothing: there is no single answer, and guessing one + * would mislabel the others. + * + * @param {Array<{ code?: string, status?: number }>} failedItems + * @returns {{ code?: string, status?: number }} + */ +export const sharedFailureFields = (failedItems) => { + const shared = {}; + for ( const field of ['code', 'status'] ) { + const values = new Set(failedItems.map((item) => item[field])); + if ( values.size === 1 && !values.has(undefined) ) { + shared[field] = [...values][0]; + } + } + return shared; +}; + /** * Resolve the destination path for a signed batch request item, used for * reporting which items failed. @@ -675,10 +705,13 @@ export async function performSignedBatchUpload (ctx) { ? path.basename(itemPath) : undefined, message: toErrorMessage(item.error), + code: typeof item.error?.code === 'string' ? item.error.code : undefined, + status: typeof item.error?.status === 'number' ? item.error.status : undefined, }; }); partialError.partial = true; partialError.failedItems = mappedFailedSignedItems; + Object.assign(partialError, sharedFailureFields(mappedFailedSignedItems)); partialError.failedPaths = mappedFailedSignedItems .map((item) => item.path) .filter((itemPath) => typeof itemPath === 'string' && itemPath.length > 0); diff --git a/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.test.js b/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.test.js new file mode 100644 index 000000000..c9749eaa1 --- /dev/null +++ b/src/puter-js/src/modules/FileSystem/operations/upload/signedBatchUpload.test.js @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest'; + +import { + isSignedBatchWriteUnavailableError, + sharedFailureFields, +} from './signedBatchUpload.js'; + +// An over-quota account fails every item of the batch with the same rejection. +// Callers — including `upload`'s own error handler, which is what asks the user +// to free up space — key on `code`/`status`, so those have to survive the trip +// through the partial-failure error. +describe('sharedFailureFields', () => { + it('reports the code and status every failed item shares', () => { + const failed = [ + { code: 'storage_limit_reached', status: 413 }, + { code: 'storage_limit_reached', status: 413 }, + ]; + expect(sharedFailureFields(failed)).toEqual({ + code: 'storage_limit_reached', + status: 413, + }); + }); + + it('reports nothing when the failures disagree', () => { + const failed = [ + { code: 'storage_limit_reached', status: 413 }, + { code: 'forbidden', status: 403 }, + ]; + expect(sharedFailureFields(failed)).toEqual({}); + }); + + it('reports nothing when a failure carries no code at all', () => { + const failed = [ + { code: 'storage_limit_reached', status: 413 }, + { code: undefined, status: undefined }, + ]; + expect(sharedFailureFields(failed)).toEqual({}); + }); + + it('still reports a status when the items carry no codes', () => { + const failed = [{ status: 413 }, { status: 413 }]; + expect(sharedFailureFields(failed)).toEqual({ status: 413 }); + }); +}); + +describe('isSignedBatchWriteUnavailableError', () => { + it('treats a partial failure as a working endpoint', () => { + // 404 is otherwise read as "this backend has no signed batch writes". + // On a partial error it is the items' status, and retrying the whole + // batch on the legacy path would only fail again. + const partial = Object.assign(new Error('partial'), { + partial: true, + status: 404, + }); + expect(isSignedBatchWriteUnavailableError(partial)).toBe(false); + }); + + it('still detects a missing endpoint', () => { + expect( + isSignedBatchWriteUnavailableError( + Object.assign(new Error('nope'), { status: 404 }), + ), + ).toBe(true); + }); + + it('does not treat a structured error as a missing endpoint', () => { + expect( + isSignedBatchWriteUnavailableError( + Object.assign(new Error('nope'), { + status: 404, + body: { code: 'subject_does_not_exist' }, + }), + ), + ).toBe(false); + }); +});