add fixes 1 (#3458)

* add fixes 1

* change socketio test

* clarify that success/error callbacks are legacy

* fix wisp issues
This commit is contained in:
Neal Shah
2026-07-28 17:34:12 -04:00
committed by GitHub
parent 1db4324c02
commit 0a61b4e78b
11 changed files with 541 additions and 19 deletions
+1 -1
View File
@@ -988,7 +988,7 @@ const puterInit = function () {
};
resetAuthToken = function () {
if (this.env === 'worker' || this.env === 'service-worker') {
if (this.env === 'web-worker' || this.env === 'service-worker') {
throw new Error(
'Sign out is not permitted from WebWorkers or ServiceWorkers',
);
+11 -4
View File
@@ -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];
}
+116
View File
@@ -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' });
});
});
});
@@ -1,8 +1,8 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
// The module opens a socket in its constructor; nothing here exercises it.
vi.mock('../../lib/socket.io/socket.io.esm.min.js', () => ({
default: () => ({ on: vi.fn(), disconnect: vi.fn() }),
vi.mock('socket.io-client', () => ({
io: () => ({ on: vi.fn(), disconnect: vi.fn() }),
}));
const { PuterJSFileSystemModule } = await import('./index.js');
-3
View File
@@ -207,9 +207,6 @@ export class WorkersHandler extends PuterModule {
const driverResult = await utils.makeDriverMethod({ iface: 'workers', driver: 'worker-service', method: 'destroy', argNames: ['authorization', 'workerName'] })(this.puter.authToken, workerName);
if ( ! driverResult.result ) {
if ( ! driverResult.result ) {
new Error("Worker doesn't exist");
}
throw new Error(driverResult?.errors || 'Driver failed to execute, do you have the necessary permissions?');
} else {
let currentWorkers = await this.puter.kv.get('user-workers');
+31
View File
@@ -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 },
+18 -5
View File
@@ -50,9 +50,17 @@ export class PSocket extends EventListener {
wispInfo.handler = new PWispHandler(wispServer, wispToken);
// Wait for websocket to fully open
await new Promise((res, req) => {
wispInfo.handler.onReady = res;
});
try {
await new Promise((res, rej) => {
wispInfo.handler.onReady = res;
wispInfo.handler.onError = rej;
});
} catch (e) {
// Drop the dead handler so the next socket redials instead
// of registering streams on a relay that never opened.
wispInfo.handler = undefined;
throw e;
}
}
const callbacks = {
@@ -74,7 +82,12 @@ export class PSocket extends EventListener {
this.emit('open', undefined);
}, 0);
})();
})().catch((e) => {
// Nothing awaits this body, so a failure to connect has to reach
// the caller as an 'error' event rather than an unhandled rejection.
this.emit('error', e instanceof Error ? e : new Error(String(e)));
this.emit('close', true);
});
}
/**
* Registers a handler for a socket event, the same as `on`.
@@ -100,7 +113,7 @@ export class PSocket extends EventListener {
wispInfo.handler.write(this._streamID, data);
if ( callback ) callback();
} else if ( data.resize ) { // ArrayBuffer
data.write(this._streamID, new Uint8Array(data));
wispInfo.handler.write(this._streamID, new Uint8Array(data));
if ( callback ) callback();
} else if ( typeof (data) === 'string' ) {
wispInfo.handler.write(this._streamID, texten.encode(data));
@@ -0,0 +1,161 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
// The relay handshake is the thing under test, so both the token fetch and the
// wisp handler are stubbed: no network, no websocket.
const mockFetchUrl = vi.fn();
vi.mock('../../lib/networkUtils.js', () => ({
fetchUrl: (...args) => mockFetchUrl(...args),
}));
const handlerInstances = [];
class FakeWispHandler {
onReady = undefined;
onError = undefined;
constructor (url, auth) {
this.url = url;
this.auth = auth;
this.write = vi.fn();
this.close = vi.fn();
this.register = vi.fn(() => 7);
handlerInstances.push(this);
}
}
vi.mock('./PWispHandler.js', () => ({
PWispHandler: class {
constructor (...args) {
return new FakeWispHandler(...args);
}
},
}));
const { PSocket, wispInfo } = await import('./PSocket.js');
const tokenResponse = () => Promise.resolve({
json: () => Promise.resolve({ token: 'wisp-token', server: 'wss://relay.test/' }),
});
// Lets a test act after the constructor's async body has reached its await.
const flush = () => new Promise(r => setTimeout(r, 0));
const origPuter = globalThis.puter;
beforeEach(() => {
handlerInstances.length = 0;
mockFetchUrl.mockReset().mockImplementation(tokenResponse);
wispInfo.handler = undefined;
globalThis.puter = { authToken: 'tok', APIOrigin: 'https://api.test', env: 'nodejs' };
});
afterEach(() => {
wispInfo.handler = undefined;
globalThis.puter = origPuter;
});
describe('PSocket relay handshake', () => {
it('opens once the handler reports ready', async () => {
const socket = new PSocket('example.com', 80);
const onOpen = vi.fn();
socket.on('open', onOpen);
await flush();
handlerInstances[0].onReady();
// `open` is emitted from a setTimeout of its own, one tick after the
// constructor body resumes.
await flush();
await flush();
expect(onOpen).toHaveBeenCalledTimes(1);
expect(handlerInstances[0].register).toHaveBeenCalledWith(
'example.com', 80, expect.any(Object),
);
});
// The reject parameter was typo'd `req` and never called, so a failed
// handshake hung instead of surfacing.
it('emits error instead of hanging when the handshake fails', async () => {
const socket = new PSocket('example.com', 80);
const onError = vi.fn();
const onClose = vi.fn();
socket.on('error', onError);
socket.on('close', onClose);
await flush();
handlerInstances[0].onError(new Error('relay unreachable'));
await flush();
expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0]).toBeInstanceOf(Error);
expect(onError.mock.calls[0][0].message).toBe('relay unreachable');
expect(onClose).toHaveBeenCalledWith(true);
// The stream is never registered on a relay that failed to open.
expect(handlerInstances[0].register).not.toHaveBeenCalled();
});
it('drops the dead handler so a later socket redials', async () => {
const first = new PSocket('example.com', 80);
first.on('error', () => {});
await flush();
handlerInstances[0].onError(new Error('relay unreachable'));
await flush();
expect(wispInfo.handler).toBeUndefined();
const second = new PSocket('example.com', 80);
second.on('error', () => {});
await flush();
expect(handlerInstances).toHaveLength(2);
});
});
describe('PSocket write', () => {
const connected = async () => {
const socket = new PSocket('example.com', 80);
socket.on('error', () => {});
await flush();
handlerInstances[0].onReady();
await flush();
return socket;
};
it('writes a typed array through the relay handler', async () => {
const socket = await connected();
const payload = new Uint8Array([1, 2, 3]);
socket.write(payload);
expect(handlerInstances[0].write).toHaveBeenCalledWith(7, payload);
});
// This branch called `data.write(...)` — a method ArrayBuffers do not have
// — so every ArrayBuffer write threw instead of reaching the relay.
it('writes an ArrayBuffer through the relay handler', async () => {
const socket = await connected();
const buffer = new Uint8Array([4, 5, 6]).buffer;
const callback = vi.fn();
expect(() => socket.write(buffer, callback)).not.toThrow();
expect(handlerInstances[0].write).toHaveBeenCalledTimes(1);
const [streamID, sent] = handlerInstances[0].write.mock.calls[0];
expect(streamID).toBe(7);
expect(Array.from(sent)).toEqual([4, 5, 6]);
expect(callback).toHaveBeenCalledTimes(1);
});
it('encodes a string before writing', async () => {
const socket = await connected();
socket.write('hi');
const [, sent] = handlerInstances[0].write.mock.calls[0];
expect(Array.from(sent)).toEqual([104, 105]);
});
it('throws on an unsupported data type', async () => {
const socket = await connected();
expect(() => socket.write(42)).toThrow(/Invalid data type/);
});
});
@@ -4,12 +4,29 @@ export class PWispHandler {
_ws;
_nextStreamID = 1;
_bufferMax;
// Set once the relay answers the handshake with CONTINUE on stream 0.
// Decides whether a close is a dropped connection (reconnect) or a
// handshake that never completed (report failure).
_ready = false;
onReady = undefined;
onError = undefined;
streamMap = new Map();
constructor (wispURL, puterAuth) {
const setup = () => {
this._ws = new WebSocket(wispURL);
this._ws.binaryType = 'arraybuffer';
this._ws.onerror = () => {
this._fail(new Error(`Wisp relay connection failed: ${wispURL}`));
};
this._ws.onclose = () => {
if ( this._ready ) {
// Pass the function itself: `setTimeout(setup(), 1000)`
// would reconnect immediately and schedule nothing.
setTimeout(setup, 1000);
return;
}
this._fail(new Error('Wisp relay closed before the handshake completed'));
};
this._ws.onmessage = (event) => {
const parsed = parseIncomingPacket(new Uint8Array(event.data));
switch ( parsed.packetType ) {
@@ -19,16 +36,14 @@ export class PWispHandler {
case CONTINUE:
if ( parsed.streamID === 0 ) {
this._bufferMax = parsed.remainingBuffer;
this._ws.onclose = () => {
setTimeout(setup(), 1000);
};
this._ready = true;
if ( this.onReady ) {
this.onReady();
}
return;
}
this.streamMap.get(parsed.streamID).buffer = parsed.remainingBuffer;
this._continue();
this._continue(parsed.streamID);
break;
case CLOSE:
if ( parsed.streamID !== 0 )
@@ -48,6 +63,13 @@ export class PWispHandler {
};
setup();
}
// Reports a connection-level failure once, so a caller waiting on the
// handshake is rejected rather than left hanging.
_fail (error) {
const onError = this.onError;
this.onError = undefined;
if ( onError ) onError(error);
}
_continue (streamID) {
const queue = this.streamMap.get(streamID).queue;
for ( let i = 0; i < queue.length; i++ ) {
@@ -0,0 +1,151 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { CONTINUE, DATA, createWispPacket, parseIncomingPacket } from './parsers.js';
import { PWispHandler } from './PWispHandler.js';
// Fake WebSocket standing in for the relay connection. Records every instance
// so reconnects are observable, and every frame sent so writes are.
class FakeWebSocket {
static instances = [];
constructor (url) {
this.url = url;
this.sent = [];
this.readyState = 0;
FakeWebSocket.instances.push(this);
}
send (data) {
this.sent.push(data);
}
close () {}
}
const latest = () => FakeWebSocket.instances.at(-1);
// Hand a wisp packet to the handler the way the browser would.
const deliver = (packet) => latest().onmessage({ data: packet.buffer });
const handshake = (remainingBuffer = 4) =>
deliver(createWispPacket({ packetType: CONTINUE, streamID: 0, remainingBuffer }));
const origWebSocket = globalThis.WebSocket;
beforeEach(() => {
vi.useFakeTimers();
FakeWebSocket.instances = [];
globalThis.WebSocket = FakeWebSocket;
});
afterEach(() => {
vi.useRealTimers();
globalThis.WebSocket = origWebSocket;
});
describe('PWispHandler handshake', () => {
it('signals ready once the relay answers CONTINUE on stream 0', () => {
const handler = new PWispHandler('wss://relay.test/', 'token');
const onReady = vi.fn();
handler.onReady = onReady;
handshake(7);
expect(onReady).toHaveBeenCalledTimes(1);
expect(handler._bufferMax).toBe(7);
});
// Without an `onerror` handler a failed connection left every caller
// waiting on the handshake hanging forever.
it('reports a connection error to onError', () => {
const handler = new PWispHandler('wss://relay.test/', 'token');
const onError = vi.fn();
handler.onError = onError;
expect(typeof latest().onerror).toBe('function');
latest().onerror(new Event('error'));
expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0]).toBeInstanceOf(Error);
});
it('reports a close that happens before the handshake completes', () => {
const handler = new PWispHandler('wss://relay.test/', 'token');
const onError = vi.fn();
handler.onError = onError;
latest().onclose();
expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0].message).toMatch(/before the handshake/);
});
it('reports a connection failure only once', () => {
const handler = new PWispHandler('wss://relay.test/', 'token');
const onError = vi.fn();
handler.onError = onError;
latest().onerror(new Event('error'));
latest().onclose();
expect(onError).toHaveBeenCalledTimes(1);
});
});
describe('PWispHandler reconnect', () => {
// `setTimeout(setup(), 1000)` reconnected synchronously and scheduled
// `undefined`, so the 1s backoff never applied.
it('reconnects one second after an established connection drops', () => {
new PWispHandler('wss://relay.test/', 'token');
handshake();
expect(FakeWebSocket.instances).toHaveLength(1);
latest().onclose();
// Nothing yet: the reconnect is scheduled, not immediate.
expect(FakeWebSocket.instances).toHaveLength(1);
vi.advanceTimersByTime(1000);
expect(FakeWebSocket.instances).toHaveLength(2);
expect(latest().url).toBe('wss://relay.test/');
});
it('does not reconnect when the handshake never completed', () => {
new PWispHandler('wss://relay.test/', 'token');
latest().onclose();
vi.advanceTimersByTime(5000);
expect(FakeWebSocket.instances).toHaveLength(1);
});
});
describe('PWispHandler backpressure', () => {
// `this._continue()` was called with no argument, so the drain path did
// `streamMap.get(undefined).queue` and threw.
it('drains the queue for the stream the CONTINUE names', () => {
const handler = new PWispHandler('wss://relay.test/', 'token');
handshake(1);
const streamID = handler.register('example.com', 80, {
dataCallBack: vi.fn(), closeCallBack: vi.fn(),
});
const sentAfterRegister = latest().sent.length;
// Exhaust the credit, so the next write is queued rather than sent.
handler.write(streamID, new Uint8Array([1]));
handler.write(streamID, new Uint8Array([2]));
expect(handler.streamMap.get(streamID).queue).toHaveLength(1);
expect(latest().sent).toHaveLength(sentAfterRegister + 1);
expect(() => deliver(createWispPacket({
packetType: CONTINUE, streamID, remainingBuffer: 4,
}))).not.toThrow();
expect(handler.streamMap.get(streamID).queue).toHaveLength(0);
const flushed = parseIncomingPacket(latest().sent.at(-1));
expect(flushed.packetType).toBe(DATA);
expect(flushed.streamID).toBe(streamID);
expect(Array.from(flushed.payload)).toEqual([2]);
});
});
@@ -33,6 +33,30 @@ export default suite('auth', {
},
},
// Both worker environments must refuse signOut. No platform here reports
// 'web-worker' (workerd reports 'service-worker'), so the env is forced to
// reach the guard for each value it is meant to cover.
'signOut is refused in every worker environment': async (t) => {
const realEnv = t.puter.env;
try {
for (const env of ['web-worker', 'service-worker'] as const) {
t.puter.env = env;
await t.assert.rejects(
async () => t.puter.auth.signOut(),
`signOut should be refused when env is ${env}`,
);
t.assert.equal(
t.puter.auth.isSignedIn(),
true,
`a refused signOut must leave the token intact (env ${env})`,
);
}
} finally {
t.puter.env = realEnv;
t.puter.setAuthToken(t.env.users.user.token);
}
},
'a bogus token is rejected by the API': async (t) => {
const res = await fetch(`${t.env.apiOrigin}/whoami`, {
headers: {