From e0ce0597c85d2aa43ff0a6da22e8abf9de6653a1 Mon Sep 17 00:00:00 2001 From: ProgrammerIn-wonderland <3838shah@gmail.com> Date: Tue, 28 Jul 2026 17:04:12 -0400 Subject: [PATCH] clarify that success/error callbacks are legacy --- src/puter-js/src/lib/utils.js | 15 +++- src/puter-js/src/lib/utils.test.js | 116 +++++++++++++++++++++++++ src/puter-js/src/modules/kv/kv.test.js | 31 +++++++ 3 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 src/puter-js/src/lib/utils.test.js diff --git a/src/puter-js/src/lib/utils.js b/src/puter-js/src/lib/utils.js index a4841e6d7..5fe34f7c2 100644 --- a/src/puter-js/src/lib/utils.js +++ b/src/puter-js/src/lib/utils.js @@ -201,10 +201,15 @@ function setupXhrEventHandlers (xhr, success_cb, error_cb, resolve_func, reject_ } /** - * Makes the hybrid promise/callback function for one driver method: the - * returned function takes either a named-parameters object or the positional - * arguments listed in `argNames`, optionally followed by success/error - * callbacks, and resolves the driver's `result`. + * Makes the function for one driver method: the returned function takes either + * a named-parameters object or the positional arguments listed in `argNames`, + * optionally followed by legacy success/error callbacks, and resolves the + * driver's `result`. + * + * `error` is forwarded to `driverCall` as `onError`; `success` is consumed so + * it stays off the wire but is never invoked — these methods are promise-only. + * That is deliberate: it has never fired, so invoking it now would double-run + * handlers in apps that pass one and also await the promise. * * @param {{ * iface: string, @@ -240,6 +245,8 @@ function makeDriverMethod (spec) { argNames.forEach((argName, index) => { driverArgs[argName] = args[index]; }); + // `argNames.length` is the legacy success slot, deliberately + // skipped; the error callback follows it. onError = args[argNames.length + 1]; } diff --git a/src/puter-js/src/lib/utils.test.js b/src/puter-js/src/lib/utils.test.js new file mode 100644 index 000000000..27974fbb7 --- /dev/null +++ b/src/puter-js/src/lib/utils.test.js @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { makeDriverMethod } from './utils.js'; + +/** + * Pins the callback contract of `makeDriverMethod`: driver methods are + * promise-only. A legacy `error` callback is honored, a legacy `success` + * callback is consumed but never invoked, and neither ever reaches the wire. + * + * The success drop is deliberate, not an oversight — it has never fired, so + * invoking it now would start double-running handlers in apps that pass one and + * also await the promise. `puter.fs.*` has its own working implementation + * (`modules/FileSystem/operations/scaffold.js`) and is unaffected. + */ + +// Minimal XHR fake: replays one driver-layer response for every request. +function installFakeXHR (respObj) { + const requests = []; + class FakeXHR { + _listeners = {}; + responseType = ''; + status = 200; + open (method, url) { this.method = method; this.url = url; } + setRequestHeader () {} + addEventListener (type, fn) { (this._listeners[type] ??= []).push(fn); } + getResponseHeader () { return null; } + send (body) { + requests.push(this); + this.requestBody = body; + queueMicrotask(() => { + this.responseText = JSON.stringify(respObj); + for ( const fn of this._listeners.load ?? [] ) fn.call(this, { target: this }); + }); + } + } + globalThis.XMLHttpRequest = FakeXHR; + return requests; +} + +const wireArgs = requests => JSON.parse(requests.at(-1).requestBody).args; + +const ok = { success: true, result: 'the-result' }; +const driverError = { success: false, error: { code: 'nope' } }; + +let savedXHR; +beforeEach(() => { + savedXHR = globalThis.XMLHttpRequest; + globalThis.puter = { authToken: 'tok', APIOrigin: 'https://api.test', env: 'nodejs' }; +}); +afterEach(() => { + globalThis.XMLHttpRequest = savedXHR; + delete globalThis.puter; + vi.restoreAllMocks(); +}); + +const makeMethod = () => makeDriverMethod({ + iface: 'test-iface', driver: 'test-driver', method: 'doThing', argNames: ['key'], +}); + +describe('makeDriverMethod legacy callbacks', () => { + describe('positional form', () => { + it('resolves the driver result without invoking a success callback', async () => { + const requests = installFakeXHR(ok); + const success = vi.fn(); + + await expect(makeMethod()('k', success)).resolves.toBe('the-result'); + + expect(success).not.toHaveBeenCalled(); + expect(wireArgs(requests)).toEqual({ key: 'k' }); + }); + + it('invokes the error callback that follows the success slot', async () => { + installFakeXHR(driverError); + const success = vi.fn(); + const error = vi.fn(); + + await expect(makeMethod()('k', success, error)).rejects.toEqual(driverError); + + expect(error).toHaveBeenCalledWith(driverError); + expect(success).not.toHaveBeenCalled(); + }); + + it('still finds the error callback when the success slot is empty', async () => { + installFakeXHR(driverError); + const error = vi.fn(); + + await expect(makeMethod()('k', undefined, error)).rejects.toEqual(driverError); + + expect(error).toHaveBeenCalledWith(driverError); + }); + }); + + describe('named-parameters form', () => { + it('resolves the driver result without invoking a success callback', async () => { + const requests = installFakeXHR(ok); + const success = vi.fn(); + + await expect(makeMethod()({ key: 'k', success })).resolves.toBe('the-result'); + + expect(success).not.toHaveBeenCalled(); + // Callbacks must never be serialized into the request. + expect(wireArgs(requests)).toEqual({ key: 'k' }); + }); + + it('invokes the error callback and keeps both callbacks off the wire', async () => { + const requests = installFakeXHR(driverError); + const success = vi.fn(); + const error = vi.fn(); + + await expect(makeMethod()({ key: 'k', success, error })).rejects.toEqual(driverError); + + expect(error).toHaveBeenCalledWith(driverError); + expect(success).not.toHaveBeenCalled(); + expect(wireArgs(requests)).toEqual({ key: 'k' }); + }); + }); +}); diff --git a/src/puter-js/src/modules/kv/kv.test.js b/src/puter-js/src/modules/kv/kv.test.js index d76fad0ef..a591743ec 100644 --- a/src/puter-js/src/modules/kv/kv.test.js +++ b/src/puter-js/src/modules/kv/kv.test.js @@ -124,6 +124,37 @@ describe('kv.set driver payloads', () => { expect(lastBody().args).toEqual({ key: 'k', value: 'v' }); }); + // kv routes through `makeDriverMethod`, which is promise-only: it honors a + // legacy `error` callback but never invokes `success`. Wiring `success` up + // would start double-running handlers in apps that pass one and also await + // the promise, so the drop is pinned here deliberately. + it('does not invoke a trailing success callback', async () => { + const success = vi.fn(); + await expect(kv.set('k', 'v', success)).resolves.toBe(true); + expect(success).not.toHaveBeenCalled(); + }); + + it('does not invoke a success callback passed in the object form', async () => { + const success = vi.fn(); + await expect(kv.set({ key: 'k', value: 'v', success })).resolves.toBe(true); + expect(success).not.toHaveBeenCalled(); + expect(lastBody().args).toEqual({ key: 'k', value: 'v' }); + }); + + it('still invokes a trailing error callback on a driver error', async () => { + FakeXHR.respondWith = () => ({ success: false, error: { code: 'key_too_large' } }); + const success = vi.fn(); + const error = vi.fn(); + + await expect(kv.set('k', 'v', success, error)).rejects.toEqual({ + success: false, error: { code: 'key_too_large' }, + }); + expect(error).toHaveBeenCalledWith({ + success: false, error: { code: 'key_too_large' }, + }); + expect(success).not.toHaveBeenCalled(); + }); + it('set([items]) becomes a batchPut with normalized items', async () => { await kv.set([ { key: 'a', value: 1 },