diff --git a/src/backend/controllers/events/limits.ts b/src/backend/controllers/events/limits.ts index aecefeade..58c3ad62b 100644 --- a/src/backend/controllers/events/limits.ts +++ b/src/backend/controllers/events/limits.ts @@ -155,6 +155,13 @@ export const EVENTS_LIST_LIMIT = userWindow('events:list', 120); */ export const EVENTS_ACK_LIMIT = userWindow('events:ack', 600); +/** + * How long a connected client holds a queued delivery before the events worker + * may take it. A client that has no handler for it, or whose handler fails, + * declines it at once; this only bounds one that never answers. + */ +export const EVENTS_SOCKET_LEASE_MS = 15_000; + /** * Share-handle mint + revoke calls per minute, per user. * diff --git a/src/backend/services/events/EventForwardService.ts b/src/backend/services/events/EventForwardService.ts index 35ad0730e..8a00c30d1 100644 --- a/src/backend/services/events/EventForwardService.ts +++ b/src/backend/services/events/EventForwardService.ts @@ -33,6 +33,7 @@ import { FORWARD_MAX_QUEUED_BYTES, PeerForwardQueue, type ForwardAck, + type ForwardNack, type ForwardBatch, type ForwardBump, type ForwardDelivery, @@ -100,7 +101,7 @@ export interface ForwardableDelivery { event: DeliverableEvent; ackRequired?: true; ackId?: string; - /** Client skips its handler: the worker has it, or it is too deep. */ + /** Client skips its handler: the event is past the handler-chain limit. */ skipHandler?: true; } @@ -440,6 +441,25 @@ export class EventForwardService extends PuterService { }); } + /** Relay a client's decline to the region that owns the lease. */ + relayNack( + region: string, + userId: number, + subId: string, + entryId: string, + reason: ForwardNack['reason'], + ): void { + if (!this.active) return; + if (!this.services.broadcast.addressablePeers.includes(region)) return; + this.#queueFor().push(region, { + kind: 'nack', + userId, + subId, + entryId, + reason, + }); + } + /** Whether a region name is a peer this node can address. */ isPeer(region: string): boolean { return this.services.broadcast.addressablePeers.includes(region); @@ -664,6 +684,26 @@ export class EventForwardService extends PuterService { if (outcome.status === 'rejected') console.warn('[events] relayed ack failed', outcome.reason); + const nacks = items.filter( + (item): item is ForwardNack => item.kind === 'nack', + ); + const declined = await runWithConcurrencyLimitSettled( + nacks, + EventForwardService.RECEIVE_CONCURRENCY, + (nack) => + this.services.events.declineRelayed( + nack.userId, + nack.subId, + nack.entryId, + nack.reason === 'retriable' || nack.reason === 'terminal' + ? nack.reason + : 'no_handler', + ), + ); + for (const outcome of declined) + if (outcome.status === 'rejected') + console.warn('[events] relayed decline failed', outcome.reason); + // One check per (user, app) pair in the batch, not per item — a busy // subscription can carry many deliveries for the same pair in one // window. diff --git a/src/backend/services/events/EventsService.test.ts b/src/backend/services/events/EventsService.test.ts index 6a9d0281c..b08b79950 100644 --- a/src/backend/services/events/EventsService.test.ts +++ b/src/backend/services/events/EventsService.test.ts @@ -52,6 +52,7 @@ import { type DeliveryEnvelope, type EventSocket, type KvDispatchOptions, + type SubscribeRequest, } from './EventsService.js'; import { kvSharePermission } from '../kvShare/kvShares.js'; import { FILTER_EVALUATIONS_PER_EVENT } from './matcher.js'; @@ -589,25 +590,18 @@ describe('subscribing', () => { anchor: { uid: documents.uid, path: documents.path }, match: null, op: null, - // The connection is the only thing a session row can be delivered - // to, so it is the only transport it can ask for. - targets: ['socket'], }); }); - it('refuses a session subscription a connection could not carry', async () => { + it('ignores `targets` on a session subscription', async () => { seedTree(); - for (const targets of [['worker'], ['socket', 'push']]) - await expect( - service.subscribe(actorFor(), socketId, { - subject: `fs:/u${userId}/Documents`, - targets, - }), - ).rejects.toSatisfy( - (err: unknown) => - isHttpError(err) && err.legacyCode === 'invalid_targets', - ); + const { sub } = await service.subscribe(actorFor(), socketId, { + subject: `fs:/u${userId}/Documents`, + targets: ['worker', 'push'], + } as SubscribeRequest); + + expect(sub.subId).toEqual(expect.any(String)); }); it('rejects a subject that resolves to nothing', async () => { @@ -1224,7 +1218,6 @@ describe('matching', () => { permission: 'list', durable: true, delivery: 'broadcast', - targets: ['socket'], handlerName: null, context: null, expiresAt: null, @@ -1809,7 +1802,6 @@ describe('limits', () => { appUid: null, permission: 'list', delivery: 'broadcast', - targets: ['socket'], handlerName: null, context: null, expiresAt: null, diff --git a/src/backend/services/events/EventsService.ts b/src/backend/services/events/EventsService.ts index 18cd5645f..97bb77f47 100644 --- a/src/backend/services/events/EventsService.ts +++ b/src/backend/services/events/EventsService.ts @@ -83,12 +83,8 @@ import type { PendingShed, } from '../../stores/events/PendingDeliveryStore.js'; import { - DEFAULT_DURABLE_TARGETS, - SESSION_TARGETS, - isSubscriptionTarget, - targetsAllowedForDelivery, + runsInWorker, type SubscriptionPermission, - type SubscriptionTarget, } from '../../stores/events/types.js'; import type { FSEntry } from '../../stores/fs/FSEntry.js'; import { parseKvNamespace } from '../../stores/systemKv/SystemKVStore.js'; @@ -140,7 +136,6 @@ import { EVENTS_BACKGROUND_PERMISSION, kvShareHandleDisabled, kvSharedRegionAuthorized, - needsBackgroundConsent, nodeDescriptor, resolveGrantActor, rowInActorScope, @@ -263,7 +258,6 @@ import { export interface SubscribeRequest { subject?: unknown; - targets?: unknown; /** KV subjects only: deliver the key's new value alongside the key. */ includeValue?: unknown; } @@ -284,10 +278,30 @@ export interface AckRequest { origin?: unknown; } +/** + * Why a client handed a queued delivery back: it has no handler for it, or its + * handler failed in a way worth retrying, or in a way that is not. + */ +export type DeclineReason = 'no_handler' | 'retriable' | 'terminal'; + +/** Body of the `events.nack` verb: an ack's fields, plus why. */ +export interface NackRequest extends AckRequest { + /** `failed` is a handler failure; anything else is "no handler here". */ + reason?: unknown; + /** For a failure: whether it is worth trying again. */ + retry?: unknown; +} + +const declineReason = (request: NackRequest): DeclineReason => { + if (request?.reason !== 'failed') return 'no_handler'; + return request.retry === true ? 'retriable' : 'terminal'; +}; + /** Body of `POST /events/subscribe`. */ export interface DurableSubscribeRequest extends SubscribeRequest { + queue?: unknown; + /** Older form of `queue`: `'single'` or `'broadcast'`. */ delivery?: unknown; - targets?: unknown; handlerName?: unknown; /** * Hash of the source the caller believes `handlerName` is published with. @@ -395,7 +409,6 @@ export interface SubscriptionView { anchor: { uid: string; path: string }; match: string | null; op: FsOp | null; - targets: SubscriptionTarget[]; /** Whether KV deliveries on this row carry the key's new value. */ includeValue: boolean; } @@ -409,7 +422,8 @@ export interface SubscriptionView { * two subscriptions apart without being handed either one's secrets. */ export interface DurableSubscriptionView extends SubscriptionView { - delivery: DeliveryClass; + /** Each event is leased to one consumer and redelivered until acked. */ + queue: boolean; handlerName: string | null; appUid: string | null; /** Key names of the stored context, or `null` for a row without one. */ @@ -442,19 +456,17 @@ export interface DeliveryEnvelope { * receives it knows where to send it. */ origin?: string; - /** Client skips its handler: the worker has it, or it is too deep. */ + /** Client skips its handler: the event is past the handler-chain limit. */ skipHandler?: true; } /** * The envelope plus where it goes. The address is not part of the wire, and - * neither is the handler a durable row may want run alongside the socket copy. + * neither is the handler a durable row runs alongside the socket copy. */ interface AddressedDelivery { target: SocketSpecifier; envelope: DeliveryEnvelope; - /** False for a row that asked for its handler and no socket copy. */ - socket: boolean; /** * Whether the socket copy also goes to the other regions holding one. True * for `broadcast`, which every connected subscriber gets; a `single` picks @@ -614,6 +626,8 @@ interface RouteLimits { export const EVENTS_SUBSCRIBE_VERB = 'events.subscribe'; export const EVENTS_UNSUBSCRIBE_VERB = 'events.unsubscribe'; export const EVENTS_ACK_VERB = 'events.ack'; +/** Client→server: hand a queued delivery back, so the worker takes it now. */ +export const EVENTS_NACK_VERB = 'events.nack'; export const EVENTS_DELIVERY_CHANNEL = 'events.delivery'; /** Server→client, no ack: the server ended a session subscription itself. */ export const EVENTS_ENDED_CHANNEL = 'events.ended'; @@ -665,11 +679,11 @@ const WORKER_SESSION_SWEEP_MAX_PAGES = 2; // -- Owed deliveries -------------------------------------------------- /** - * Socket attempts one `single` delivery gets before its handler takes it. Two, - * because the second is what a client that reconnected — or a second connection - * of the same account — is worth trying; a third is just a slower hand-off. + * Socket attempts one queued delivery gets before the worker takes it. One: a + * client with no handler for it, or whose handler fails, declines it at once, + * and a second try would only delay the worker by another socket lease. */ -const SINGLE_SOCKET_ATTEMPTS = 2; +const QUEUE_SOCKET_ATTEMPTS = 1; /** How often expired leases are reclaimed and owed deliveries retried. */ const PENDING_SWEEP_INTERVAL_MS = 10_000; @@ -732,7 +746,7 @@ const tooManyCalls = (): HttpError => }); const handlerRequired = (): HttpError => - new HttpError(400, 'A `single` subscription needs a handlerName', { + new HttpError(400, 'A `queue` subscription needs a handlerName', { legacyCode: 'events_handler_required', }); @@ -865,7 +879,6 @@ const toView = (sub: DispatchSubscription): SubscriptionView => { ? relativeToKvShareRoot(sub.permission, sub.match) : sub.match, op: sub.op, - targets: sub.targets ?? SESSION_TARGETS, includeValue: sub.includeValue === true, }; }; @@ -892,7 +905,7 @@ const projectContext = ( const toDurableView = (sub: DurableSubscription): DurableSubscriptionView => ({ ...toView(sub), - delivery: sub.delivery, + queue: sub.delivery === 'single', handlerName: sub.handlerName, appUid: sub.appUid, ...projectContext(sub.context), @@ -918,22 +931,10 @@ const deliveryTarget = (row: DispatchSubscription): SocketSpecifier => { }; }; -/** Transports a row is asking for, whichever store it came from. */ -const targetsOf = (row: DispatchSubscription): SubscriptionTarget[] => - row.durable === true - ? (row.targets ?? DEFAULT_DURABLE_TARGETS) - : SESSION_TARGETS; - /** Whether this deployment can address an events worker at all. */ const workerRuntimeReady = (config: IConfig): boolean => config.events?.workerRuntime === true && !!config.events?.internalSecret; -/** - * Whether this pass has anywhere to put the row. A `single` is queued whether - * or not anything is listening right now — that is what it is for — while a - * `broadcast` whose only transport is one this build cannot carry has no - * target, and an event with no target is dropped rather than held. - */ const nowSeconds = (): number => Math.floor(Date.now() / 1000); /** Past `expiresAt` a durable row is finished, sweep or no sweep. */ @@ -943,14 +944,6 @@ const unexpired = (row: DispatchSubscription): boolean => { return expiresAt === null || expiresAt > nowSeconds(); }; -const deliverable = (row: DispatchSubscription): boolean => { - if (row.durable !== true) return true; - if (!unexpired(row)) return false; - if (row.delivery === 'single') return true; - const targets = targetsOf(row); - return targets.includes('socket') || targets.includes('worker'); -}; - const isSingle = (row: DispatchSubscription): boolean => row.durable === true && row.delivery === 'single'; @@ -1167,7 +1160,7 @@ const KV_ROWS_PER_TOKEN = const dispatchKeeps = (forwarded: boolean | undefined) => forwarded ? (row: DispatchSubscription): boolean => row.socketId !== undefined - : deliverable; + : unexpired; /** Handler depths below this run on every plan, so no plan is looked up. */ const HANDLER_DEPTH_NEVER_BINDS = Math.min( @@ -1175,67 +1168,31 @@ const HANDLER_DEPTH_NEVER_BINDS = Math.min( ...Object.values(EVENTS_HANDLER_DEPTH.bySubscription), ); -const parseDelivery = (value: unknown): DeliveryClass => { - if (value === undefined || value === null || value === 'broadcast') - return 'broadcast'; - if (value === 'single') return 'single'; - throw badRequest(`Unknown delivery class: ${String(value)}`, 'bad_request'); -}; - /** - * There is exactly one events worker per app, so a row with no app has no - * worker to invoke — `targets` for one may only ever carry `socket`. Omitted - * targets default there quietly; an explicit ask for `worker` is refused rather - * than silently dropped, since that is the caller telling us it expected - * background delivery to exist. + * `queue` is the flag; `delivery: 'single' | 'broadcast'` is its older spelling + * and still accepted. `targets` is ignored. */ -const parseTargets = ( - value: unknown, - delivery: DeliveryClass, - appUid: string | null, -): SubscriptionTarget[] => { - if (value === undefined || value === null) - return appUid === null ? SESSION_TARGETS : DEFAULT_DURABLE_TARGETS; - if (!Array.isArray(value) || value.length === 0) +const parseDelivery = (request: DurableSubscribeRequest): DeliveryClass => { + const { queue, delivery } = request; + if (queue !== undefined && queue !== null && typeof queue !== 'boolean') + throw badRequest('queue must be a boolean', 'bad_request'); + if ( + delivery !== undefined && + delivery !== null && + delivery !== 'broadcast' && + delivery !== 'single' + ) throw badRequest( - 'targets must be a non-empty array', - 'invalid_targets', + `Unknown delivery class: ${String(delivery)}`, + 'bad_request', ); - if (!value.every(isSubscriptionTarget)) - throw badRequest('Unknown delivery target', 'invalid_targets'); - - const targets = [...new Set(value)]; - if (!targetsAllowedForDelivery(delivery, targets, appUid)) - throw badRequest( - 'A `single` subscription may not target `push`, and an app`s needs a `worker` target', - 'invalid_targets', - ); - if (appUid === null && targets.includes('worker')) - throw badRequest( - 'A subscription with no app has no events worker to target', - 'invalid_targets', - ); - return targets; -}; - -/** - * A session row is one connection, so the socket is the only transport it can - * have: a handler runs long after the connection is gone, and a device - * notification is not addressed to a connection at all. - */ -const parseSessionTargets = (value: unknown): SubscriptionTarget[] => { - if (value === undefined || value === null) return SESSION_TARGETS; - if (!Array.isArray(value) || value.length === 0) - throw badRequest( - 'targets must be a non-empty array', - 'invalid_targets', - ); - if (!value.every((target) => target === 'socket')) - throw badRequest( - 'A session subscription may only target `socket`', - 'invalid_targets', - ); - return SESSION_TARGETS; + if ( + typeof queue === 'boolean' && + delivery && + queue !== (delivery === 'single') + ) + throw badRequest('queue and delivery disagree', 'bad_request'); + return queue === true || delivery === 'single' ? 'single' : 'broadcast'; }; const parseHandlerName = (value: unknown): string | null => { @@ -1568,6 +1525,13 @@ export class EventsService extends PuterService { }); }) as (...args: never[]) => void); + socket.on(EVENTS_NACK_VERB, ((payload: NackRequest, ack: unknown) => { + void this.#answer(ack, async () => { + await this.declineDelivery(actor, payload); + return {}; + }); + }) as (...args: never[]) => void); + // Presence is per (user, app) and per region, so it is the connection // rather than the subscription that moves it — a client that has not // subscribed yet is still somewhere, and that is what a peer needs. @@ -1632,6 +1596,19 @@ export class EventsService extends PuterService { if (row.suspendedAt === null) await this.#drain(row); } + /** Apply a decline whose client gave it in another region. */ + async declineRelayed( + holderUserId: number, + subId: string, + entryId: string, + reason: DeclineReason, + ): Promise { + if (!this.enabled || !subId || !entryId) return; + const row = await this.stores.durableSubscription.getBySubId(subId); + if (!row || row.holderUserId !== holderUserId) return; + await this.#declined(row, entryId, reason); + } + async #answer( ack: unknown, run: () => Promise, @@ -1669,7 +1646,6 @@ export class EventsService extends PuterService { await this.#spendCallBudget(holderUserId); - const targets = parseSessionTargets(request?.targets); const includeValue = parseIncludeValue(request?.includeValue); const rawSubject = String(request?.subject ?? ''); if (includeValue) assertValueDeliverable(rawSubject); @@ -1688,7 +1664,6 @@ export class EventsService extends PuterService { op: anchor.op, appUid: actor.effectiveApp?.uid ?? null, permission: anchor.permission, - targets, ...(includeValue ? { includeValue } : {}), }; @@ -1778,24 +1753,23 @@ export class EventsService extends PuterService { await this.#spendCallBudget(holderUserId); - const delivery = parseDelivery(request?.delivery); + const delivery = parseDelivery(request ?? {}); const appUid = actor.effectiveApp?.uid ?? null; const limits = await this.#subscriptionQuota(actor); - const targets = parseTargets(request?.targets, delivery, appUid); const handlerName = parseHandlerName(request?.handlerName); const handlerHash = parseHandlerHash(request?.handlerHash); const context = parseContext(request?.context); const expiresAt = parseExpiresAt(request?.expiresAt); const includeValue = parseIncludeValue(request?.includeValue); - // A `single` is owed to exactly one consumer, and the handler is the - // only one that is always there to take it. + // A queued event is owed to exactly one consumer, and the handler is + // the only one that is always there to take it. if (delivery === 'single' && !handlerName) throw handlerRequired(); // Consent first: a row that would run the app's code with nobody // present is refused before anything about it is resolved or stored. if ( - needsBackgroundConsent(targets) && + runsInWorker({ appUid, handlerName }) && !(await this.#hasBackgroundConsent(actor)) ) throw backgroundConsentRequired(); @@ -1820,7 +1794,6 @@ export class EventsService extends PuterService { match: anchor.match, op: anchor.op, delivery, - targets, handlerName, context, permission: anchor.permission, @@ -2070,6 +2043,58 @@ export class EventsService extends PuterService { * not an error: at-least-once makes a duplicate ack routine. */ async ackDelivery(actor: Actor, request: AckRequest): Promise { + const { row, entryId, relayTo } = await this.#ackTarget(actor, request); + // The lease lives where the event was emitted, which is not always + // where the client ended up connected. Nothing about the delivery is + // held here to settle, so the ack goes home rather than being applied. + if (relayTo) { + this.services.eventForward.relayAck( + relayTo, + row.holderUserId, + row.subId, + entryId, + ); + return; + } + + const subId = row.subId; + await this.stores.pendingDelivery.settle(subId, entryId); + // A suspended row is not delivered to: settling what it already handed + // out must not be the trigger that hands out the next one. + if (row.suspendedAt === null) await this.#drain(row); + } + + /** + * Hand back a queued delivery a client will not take. The worker gets it + * now rather than when the client's lease lapses. A stale decline (the + * entry already settled, or handed on since) is ignored like a duplicate + * ack. + */ + async declineDelivery(actor: Actor, request: NackRequest): Promise { + const { row, entryId, relayTo } = await this.#ackTarget(actor, request); + const reason = declineReason(request); + if (relayTo) { + this.services.eventForward.relayNack( + relayTo, + row.holderUserId, + row.subId, + entryId, + reason, + ); + return; + } + await this.#declined(row, entryId, reason); + } + + /** The row an ack or decline names, and the region it belongs to. */ + async #ackTarget( + actor: Actor, + request: AckRequest, + ): Promise<{ + row: DurableSubscription; + entryId: string; + relayTo: string | null; + }> { if (!this.enabled) throw disabled(); const holderUserId = actor.user?.id; if (holderUserId === undefined) throw disabled(); @@ -2093,20 +2118,43 @@ export class EventsService extends PuterService { ) throw unknownSubscription(); - // The lease lives where the event was emitted, which is not always - // where the client ended up connected. Nothing about the delivery is - // held here to settle, so the ack goes home rather than being applied. const origin = String(request?.origin ?? ''); const forward = this.services.eventForward; - if (origin && origin !== forward.region && forward.isPeer(origin)) { - forward.relayAck(origin, holderUserId, subId, entryId); + const relayTo = + origin && origin !== forward.region && forward.isPeer(origin) + ? origin + : null; + return { row, entryId, relayTo }; + } + + /** + * Take a declined delivery off the client's lease. A failed client run + * counts like a failed worker run: retried after the backoff, or dropped. A + * client with no handler costs nothing and the worker runs it now; a row + * with no worker has nothing to hand it to, so its lease is left to lapse. + */ + async #declined( + row: DurableSubscription, + entryId: string, + reason: DeclineReason, + ): Promise { + if (reason === 'no_handler' && !runsInWorker(row)) return; + const claimed = await this.stores.pendingDelivery.takeFromSocket( + row.subId, + entryId, + ); + if (!claimed) return; + + singleAttempt.add(1, { + target: 'socket', + result: `declined-${reason}`, + }); + if (reason !== 'no_handler') { + await this.#handlerFailed(row, claimed, reason); return; } - - await this.stores.pendingDelivery.settle(subId, entryId); - // A suspended row is not delivered to: settling what it already handed - // out must not be the trigger that hands out the next one. - if (row.suspendedAt === null) await this.#drain(row); + if (row.suspendedAt !== null) return; + if (await this.#runInWorker(row, claimed)) await this.#drain(row); } // -- Handlers ---------------------------------------------------- @@ -3769,7 +3817,7 @@ export class EventsService extends PuterService { filterEvaluations: FILTER_EVALUATIONS_PER_EVENT, }, ): Promise { - const rows = candidates.filter(deliverable); + const rows = candidates.filter(unexpired); if (rows.length === 0) return; // One throwaway projection reads the op off the registry entry rather @@ -3831,19 +3879,18 @@ export class EventsService extends PuterService { continue; } - const targets = targetsOf(row); this.#queue(row, event.subject, { target: deliveryTarget(row), envelope: { subId: row.subId, event }, - socket: targets.includes('socket'), // A session row is addressed at one connection, which is the // one that made it and is therefore here. remote: row.socketId === undefined, - worker: this.#workerInvocation( - row, - event, - context.handlerDepth ?? 0, - )!, + worker: + this.#workerInvocation( + row, + event, + context.handlerDepth ?? 0, + ) ?? undefined, meter: meterFor(row), // `broadcast` is one send per delivery — no retry to dedup. bill: true, @@ -4206,12 +4253,8 @@ export class EventsService extends PuterService { void this.#owe(row, marker, context.handlerDepth ?? 0); continue; } - // A marker rides the socket; a row with none has nowhere to hear - // it, and sending nothing must not count as a delivery. - if (!targetsOf(row).includes('socket')) continue; this.#queue(row, marker.subject, { target: deliveryTarget(row), - socket: targetsOf(row).includes('socket'), remote: row.socketId === undefined, envelope: { subId: row.subId, event: marker }, meter: meterFor(row), @@ -4396,9 +4439,7 @@ export class EventsService extends PuterService { async #leftWithoutConsent( rows: readonly DurableSubscription[], ): Promise { - const background = rows.filter((row) => - needsBackgroundConsent(targetsOf(row)), - ); + const background = rows.filter(runsInWorker); if (background.length === 0) return []; const deps = this.#aclDeps(); @@ -4966,9 +5007,10 @@ export class EventsService extends PuterService { } /** - * One attempt at one owed delivery. Sockets first and one at a time, the - * handler once they are spent — a client that is there answers faster than - * anything else, and one that is not must not stall the delivery forever. + * One attempt at one owed delivery. A connected client first, the worker + * once that client declines it or its socket lease lapses — a client that + * is there answers faster than anything else, and one that is not must not + * stall the delivery. * * Returns whether the delivery settled, which is what says the next one may * go out now rather than when this lease lapses. @@ -4982,18 +5024,16 @@ export class EventsService extends PuterService { // suspension's window and goes out if the balance comes back. if (!(await this.#chargeable(meter, row.subId))) return false; - const targets = targetsOf(row); const target = deliveryTarget(row); - const hasWorkerFallback = targets.includes('worker'); + const hasWorkerFallback = runsInWorker(row); // Region-wide: `has` only sees this node's sockets, and the room send // below reaches a sibling node's just the same. const heldHere = - targets.includes('socket') && - (this.services.socket.has(target) || - (await this.services.eventForward.heldInRegion( - row.holderUserId, - row.appUid, - ))); + this.services.socket.has(target) || + (await this.services.eventForward.heldInRegion( + row.holderUserId, + row.appUid, + )); // A row with no worker to fall back to has nothing left to try once // its socket budget is spent. Without this, a socket that disappeared @@ -5001,7 +5041,7 @@ export class EventsService extends PuterService { // entry forever even after it reappears. if ( !hasWorkerFallback && - claimed.socketAttempts >= SINGLE_SOCKET_ATTEMPTS && + claimed.socketAttempts >= QUEUE_SOCKET_ATTEMPTS && heldHere ) { await this.stores.pendingDelivery.resetSocketAttempts( @@ -5011,15 +5051,12 @@ export class EventsService extends PuterService { claimed = { ...claimed, socketAttempts: 0, triedRegions: [] }; } - // Candidates in order: this region's own connection, then the regions - // presence names, most recently connected first. The attempt counter - // spans them, so two attempts is two candidates however they are split. - const socketsLeft = - targets.includes('socket') && - claimed.socketAttempts < SINGLE_SOCKET_ATTEMPTS; + // This region's own connection first, otherwise the region presence + // names, most recently connected first. + const socketsLeft = claimed.socketAttempts < QUEUE_SOCKET_ATTEMPTS; const here = socketsLeft && heldHere; const region = - socketsLeft && !(here && claimed.socketAttempts === 0) + socketsLeft && !here ? await this.services.eventForward.candidateRegion( row.holderUserId, row.appUid, @@ -5028,7 +5065,7 @@ export class EventsService extends PuterService { : null; if (here || region) { - await this.stores.pendingDelivery.recordSocketAttempt( + await this.stores.pendingDelivery.leaseToSocket( row.subId, claimed.entryId, ); @@ -5058,7 +5095,6 @@ export class EventsService extends PuterService { } else { await this.#send({ target, - socket: true, envelope, meter, bill, @@ -5077,6 +5113,18 @@ export class EventsService extends PuterService { return false; } + return this.#runInWorker(row, claimed); + } + + /** + * Run one claimed delivery in the events worker. Returns whether it + * settled, which is what says the next one may go out now. + */ + async #runInWorker( + row: DispatchSubscription, + claimed: ClaimedDelivery, + ): Promise { + const meter = meterFor(row); const invocation = this.#workerInvocation( row, claimed.event, @@ -5145,13 +5193,23 @@ export class EventsService extends PuterService { ): Promise { const pending = this.stores.pendingDelivery; try { - if (outcome === 'terminal') - await pending.discard( - row.subId, - claimed.entryId, - 'handler_rejected', - ); - else await pending.deferAfterFailure(row.subId, claimed.entryId); + if (outcome === 'terminal') { + // A run was made, so a dropped event is billed like a + // completed one, and still only once. + const bill = await this.#firstAttempt(row.subId, claimed); + if ( + await pending.discard( + row.subId, + claimed.entryId, + 'handler_rejected', + ) + ) + this.#delivered( + { subId: row.subId, event: claimed.event }, + meterFor(row), + bill, + ); + } else await pending.deferAfterFailure(row.subId, claimed.entryId); const failures = await pending.recordFailure(row.subId); if (failures < EVENTS_CONSECUTIVE_FAILURES) return; @@ -5189,8 +5247,7 @@ export class EventsService extends PuterService { event: DeliverableEvent, handlerDepth: number, ): WorkerInvocation | null { - if (row.durable !== true) return null; - if (!targetsOf(row).includes('worker')) return null; + if (row.durable !== true || !runsInWorker(row)) return null; return { subId: row.subId, holderUserId: row.holderUserId, @@ -5208,9 +5265,8 @@ export class EventsService extends PuterService { * not a token scoped to whatever grant the subscription was made under. It * can read what the app can read, and reaches the app's own KV and AppData * like any other app session would. What authorizes running that with - * nobody present is the `events:background` consent the row needed to - * target a worker at all, checked at subscribe time and re-run on every - * delivery. + * nobody present is the `events:background` consent the row needed to run a + * handler at all, checked at subscribe time and re-run on every delivery. * * The session behind the token is one row per (user, app), reused across * every delivery — the same idempotent worker session `puter.workers.*` @@ -5417,7 +5473,6 @@ export class EventsService extends PuterService { // was refusing. await this.#send({ target: delivery.target, - socket: delivery.socket, remote: delivery.remote, meter: delivery.meter, envelope: { @@ -5439,9 +5494,9 @@ export class EventsService extends PuterService { /** * Addressed at a socket id — which socket.io joins every socket to — or at * a room, so either way the adapter carries it to whichever node terminates - * the connection. For a durable row that also targets the worker, where the - * handler runs is decided before anything goes out: the socket copy carries - * `skipHandler` when the worker takes it or the event is too deep. + * the connection. A durable row with a handler also runs it in the events + * worker; connected clients run theirs as well, unless the event is past + * the handler-chain limit. */ async #send(delivery: AddressedDelivery): Promise { let gate: 'too-deep' | 'over-budget' | 'ok' | null = null; @@ -5449,67 +5504,54 @@ export class EventsService extends PuterService { try { gate = await this.#handlerGate(delivery.worker); } catch (err) { - // Unknown whether the worker could take it — fall back to the - // client running it, the same as a delivery with no worker at - // all. console.warn('[events] handler gate failed', err); } } // `available` is a deployment-wide fact (is there a worker runtime to // call at all), separate from whether this one invocation succeeds. - const toWorker = - gate === 'ok' && - (this.worker.available?.() ?? true) && - delivery.worker!.handlerName !== null; + const toWorker = gate === 'ok' && (this.worker.available?.() ?? true); // Past the depth cap nothing may run it, in the worker or a client. - const skipHandler = toWorker || gate === 'too-deep'; + const skipHandler = gate === 'too-deep'; const envelope: DeliveryEnvelope = skipHandler ? { ...delivery.envelope, skipHandler: true } : delivery.envelope; - let fannedOut: Promise | null = null; - if (delivery.socket) { - try { - void this.services.socket - .send(delivery.target, EVENTS_DELIVERY_CHANNEL, envelope) - .catch((err: unknown) => { - console.warn('[events] socket send failed', err); - }); - } catch (err) { - console.warn('[events] socket send failed', err); - } - - // Every connected subscriber gets a `broadcast`, and the ones this - // region cannot reach are wherever presence says they are. - if (delivery.remote) - fannedOut = this.#fanOut(delivery, skipHandler); + try { + void this.services.socket + .send(delivery.target, EVENTS_DELIVERY_CHANNEL, envelope) + .catch((err: unknown) => { + console.warn('[events] socket send failed', err); + }); + } catch (err) { + console.warn('[events] socket send failed', err); } + // Every connected subscriber gets a `broadcast`, and the ones this + // region cannot reach are wherever presence says they are. + const fannedOut = delivery.remote + ? this.#fanOut(delivery, skipHandler) + : null; + let invoked = false; if (toWorker) { - // At-most-once by construction: a `broadcast` invocation is never - // retried. The socket copy above was already marked for this run, - // so a failure here is not retried by the client either — only a - // settled outcome counts as delivered. + // A `broadcast` invocation is never retried. A run that finished, + // or threw an error that drops the event, counts as delivered. try { - invoked = - (await this.worker.invoke(delivery.worker!)) === 'settled'; + const outcome = await this.worker.invoke(delivery.worker!); + invoked = outcome === 'settled' || outcome === 'terminal'; } catch (err) { console.warn('[events] handler invocation failed', err); } } - // Nothing carried it, so nothing was delivered — and a delivery that - // did not happen is not billed, nor is a socket copy nobody was - // connected to take. - if (delivery.socket || invoked) - this.#delivered( - delivery.envelope, - delivery.meter, - delivery.bill && - (invoked || - (await this.#socketReached(delivery, fannedOut))), - ); + // One delivery is one line, however many transports carried it. A + // socket copy nobody was connected to take is not billed. + this.#delivered( + delivery.envelope, + delivery.meter, + delivery.bill && + (invoked || (await this.#socketReached(delivery, fannedOut))), + ); } /** Hand a `broadcast` to every other region holding a socket for it. */ @@ -5691,11 +5733,10 @@ export class EventsService extends PuterService { } /** - * Gate then invoke, for `single` — which decides between socket and worker - * itself rather than marking anything. `null` says nothing ran: a `single` - * stays owed and its lease paces the next attempt. `too-deep` says nothing - * ever will: the event is as many handler runs deep as the holder's plan - * lets a chain go. + * Gate then invoke, for `single`. `null` says nothing ran: a `single` stays + * owed and its lease paces the next attempt. `too-deep` says nothing ever + * will: the event is as many handler runs deep as the holder's plan lets a + * chain go. */ async #invokeHandler( invocation: WorkerInvocation, diff --git a/src/backend/services/events/appDeletion.integration.test.ts b/src/backend/services/events/appDeletion.integration.test.ts index c0c8a29b8..9df4b484f 100644 --- a/src/backend/services/events/appDeletion.integration.test.ts +++ b/src/backend/services/events/appDeletion.integration.test.ts @@ -157,7 +157,6 @@ const subscribeBackgroundHandler = async ( subject: `fs:${anchor}`, delivery: 'single', handlerName, - targets: ['worker'], }); expect(subscribed.status).toBe(200); return subscribed.body.subId as string; @@ -254,7 +253,6 @@ describe('an app deleted while it has events state', () => { match: null, op: null, delivery: 'single', - targets: ['worker'], handlerName: 'ingestUpload', context: null, permission: 'list', diff --git a/src/backend/services/events/authorization.ts b/src/backend/services/events/authorization.ts index 7895a1dcb..fc500bdf6 100644 --- a/src/backend/services/events/authorization.ts +++ b/src/backend/services/events/authorization.ts @@ -26,10 +26,7 @@ import { } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; import type { UserRow } from '../../stores/user/UserStore.js'; -import type { - SubscriptionPermission, - SubscriptionTarget, -} from '../../stores/events/types.js'; +import type { SubscriptionPermission } from '../../stores/events/types.js'; import { aclRefusal, memoizedDescriptor, @@ -208,11 +205,6 @@ export const checkDeliveryAuthorized = async ( */ export const EVENTS_BACKGROUND_PERMISSION = 'events:background'; -/** Whether a row's transports include one that runs code with nobody there. */ -export const needsBackgroundConsent = ( - targets: readonly SubscriptionTarget[], -): boolean => targets.includes('worker'); - export const backgroundConsentRequired = (): HttpError => new HttpError( 403, diff --git a/src/backend/services/events/durable.integration.test.ts b/src/backend/services/events/durable.integration.test.ts index e73293071..31664332d 100644 --- a/src/backend/services/events/durable.integration.test.ts +++ b/src/backend/services/events/durable.integration.test.ts @@ -102,7 +102,7 @@ const subIdsOf = (response: ApiResponse): string[] => /** * An app the user has granted `list` on the shared anchor, and consent to run - * its handler in the background — which durable rows target by default. + * its handler in the background. */ const makeApp = async ( options: { background?: boolean } = {}, @@ -250,127 +250,104 @@ describe('creating a durable subscription over HTTP', () => { expect(created.status).toBe(200); expect(created.body).toMatchObject({ subject: `fs:${anchor}`, - delivery: 'broadcast', - // No app, so no events worker to target — see the null-app - // suite below. - targets: ['socket'], + queue: false, appUid: null, }); expect(created.body.context).toBeUndefined(); + expect(created.body.targets).toBeUndefined(); + expect(created.body.delivery).toBeUndefined(); + expect(created.body.single).toBeUndefined(); }); it('registers a subscription owed to one consumer', async () => { await clearRows(); const created = await subscribe(env.users.user.token, { - delivery: 'single', + queue: true, handlerName: 'onWrite', }); expect(created.status).toBe(200); expect(created.body).toMatchObject({ + queue: true, + handlerName: 'onWrite', + }); + }); + + it('still reads the older `delivery` option', async () => { + await clearRows(); + const queued = await subscribe(env.users.user.token, { delivery: 'single', handlerName: 'onWrite', - targets: ['socket'], }); + const broadcast = await subscribe(env.users.user.token, { + delivery: 'broadcast', + }); + + expect(queued.body.queue).toBe(true); + expect(broadcast.body.queue).toBe(false); + }); + + it('accepts and ignores `targets`', async () => { + await clearRows(); + const created = await subscribe(env.users.user.token, { + queue: true, + handlerName: 'onWrite', + targets: ['push', 'carrier-pigeon'], + }); + + expect(created.status).toBe(200); + expect(created.body.queue).toBe(true); + }); + + it('refuses `queue` and `delivery` that disagree, or a `queue` that is not a boolean', async () => { + for (const body of [ + { queue: false, delivery: 'single' }, + { queue: 'yes' }, + { delivery: 'fanout' }, + ]) { + const refused = await subscribe(env.users.user.token, body); + expect(refused.status).toBe(400); + expect(refused.body.code).toBe('bad_request'); + } }); it('refuses one owed to a consumer it cannot name', async () => { - const refused = await subscribe(env.users.user.token, { - delivery: 'single', - }); - - expect(refused.status).toBe(400); - expect(refused.body.code).toBe('events_handler_required'); - }); - - it('refuses one owed to a device notification', async () => { - const refused = await subscribe(env.users.user.token, { - delivery: 'single', - handlerName: 'onWrite', - targets: ['push'], - }); - - expect(refused.status).toBe(400); - expect(refused.body.code).toBe('invalid_targets'); - }); - - it('refuses a `worker` target from an account session naming no app', async () => { - // Exactly one events worker per app: an account session has none, so - // asking for the worker target explicitly is refused rather than - // silently dropped — silently dropping it would leave the caller - // thinking background delivery was configured when it never could be. - const refused = await subscribe(env.users.user.token, { - delivery: 'single', - handlerName: 'onWrite', - targets: ['socket', 'worker'], - }); - - expect(refused.status).toBe(400); - expect(refused.body.code).toBe('invalid_targets'); + for (const body of [{ queue: true }, { delivery: 'single' }]) { + const refused = await subscribe(env.users.user.token, body); + expect(refused.status).toBe(400); + expect(refused.body.code).toBe('events_handler_required'); + } }); it('refuses background delivery an app has no consent for', async () => { // An app of its own, so nothing it holds includes the consent. const { token } = await makeApp({ background: false }); - const refused = await call('POST', '/events/subscribe', token, { - subject: `fs:${anchor}`, - delivery: 'single', - handlerName: 'onWrite', - targets: ['worker'], - }); + for (const queue of [true, false]) { + const refused = await call('POST', '/events/subscribe', token, { + subject: `fs:${anchor}`, + queue, + handlerName: 'onWrite', + }); - expect(refused.status).toBe(403); - expect(refused.body.code).toBe('events_background_consent_required'); - // Refused before the handler is even looked up: consent comes first. - expect(refused.body.message).toContain('events:background'); + expect(refused.status).toBe(403); + expect(refused.body.code).toBe( + 'events_background_consent_required', + ); + // Refused before the handler is even looked up: consent comes first. + expect(refused.body.message).toContain('events:background'); + } }); - it('requires consent for the default targets an app row gets, even with none named', async () => { - // An app of its own, so nothing it holds includes the consent. - const { token } = await makeApp({ background: false }); - - // No `targets` at all: an app row defaults to `['socket', 'worker']`, - // and that default still needs the consent — the gate runs on the - // resolved targets, not only on an explicit ask for `worker`. - const refused = await call('POST', '/events/subscribe', token, { - subject: `fs:${anchor}`, - }); - - expect(refused.status).toBe(403); - expect(refused.body.code).toBe('events_background_consent_required'); - }); - - it('needs no consent for a subscription only a connection hears', async () => { + it('needs no consent for an app subscription with no handler to run', async () => { const { token } = await makeApp({ background: false }); const created = await call('POST', '/events/subscribe', token, { subject: `fs:${anchor}`, - targets: ['socket'], }); expect(created.status).toBe(200); - expect(created.body.targets).toEqual(['socket']); - }); - - it('refuses a target outside the known set', async () => { - const refused = await subscribe(env.users.user.token, { - targets: ['socket', 'carrier-pigeon'], - }); - - expect(refused.status).toBe(400); - expect(refused.body.code).toBe('invalid_targets'); - }); - - it('refuses an app`s `single` subscription with no worker to fall back to', async () => { - const refused = await subscribe(appOneToken, { - delivery: 'single', - handlerName: 'onWrite', - targets: ['socket'], - }); - - expect(refused.status).toBe(400); - expect(refused.body.code).toBe('invalid_targets'); + expect(created.body.handlerName).toBeNull(); }); it('refuses a scoped token with no app, even on a subject it can read', async () => { @@ -380,11 +357,7 @@ describe('creating a durable subscription over HTTP', () => { }); it('refuses a scoped token an app issued, even into that app', async () => { - // Socket-only, so the background consent the token lacks is not - // what refuses it. - const refused = await subscribe(appOneAccessToken, { - targets: ['socket'], - }); + const refused = await subscribe(appOneAccessToken); expect(refused.status).toBe(403); expect(refused.body.code).toBe('events_durable_requires_account'); }); @@ -795,9 +768,9 @@ describe('where a durable delivery is addressed', () => { send.mockRestore(); }); - it('sends an unmarked copy for an app row that only targets the socket', async () => { + it('sends an unmarked copy for an app row', async () => { await clearRows(); - await subscribe(appOneToken, { targets: ['socket'] }); + await subscribe(appOneToken); const send = vi.spyOn(env.server.services.socket, 'send'); delivered.length = 0; @@ -1015,13 +988,6 @@ describe('what a plan lets an account hold', () => { uid, `fs:${entry!.uid}:list`, ); - // A durable app row targets the worker by default, so the caps are only - // reachable once the background consent behind that target is given. - await tiered.server.services.permission.grantUserAppPermission( - actor.actor!, - uid, - EVENTS_BACKGROUND_PERMISSION, - ); tieredApp = { uid, token: await tiered.server.services.auth.getUserAppToken( diff --git a/src/backend/services/events/forwardQueue.ts b/src/backend/services/events/forwardQueue.ts index 9619ce695..3a11b3562 100644 --- a/src/backend/services/events/forwardQueue.ts +++ b/src/backend/services/events/forwardQueue.ts @@ -53,7 +53,7 @@ export interface ForwardDelivery { ackId?: string; /** The region holding the lease, which is where the ack has to end up. */ origin?: string; - /** Client skips its handler: the worker has it, or it is too deep. */ + /** Client skips its handler: the event is past the handler-chain limit. */ skipHandler?: true; } @@ -65,6 +65,15 @@ export interface ForwardAck { entryId: string; } +/** A client's decline, on its way back to the region that owns the lease. */ +export interface ForwardNack { + kind: 'nack'; + userId: number; + subId: string; + entryId: string; + reason: 'no_handler' | 'retriable' | 'terminal'; +} + /** Region R now has (or no longer has) session watchers on one anchor token. */ export interface ForwardWatch { kind: 'watch'; @@ -123,7 +132,12 @@ export interface ForwardBump { } export type ForwardItem = - ForwardDelivery | ForwardAck | ForwardWatch | ForwardEvent | ForwardBump; + | ForwardDelivery + | ForwardAck + | ForwardNack + | ForwardWatch + | ForwardEvent + | ForwardBump; /** One batch, as a peer receives it. */ export interface ForwardBatch { diff --git a/src/backend/services/events/forwarding.test.ts b/src/backend/services/events/forwarding.test.ts index e326ca8af..7beede89e 100644 --- a/src/backend/services/events/forwarding.test.ts +++ b/src/backend/services/events/forwarding.test.ts @@ -40,14 +40,12 @@ import { presenceItemKey, PRESENCE_NO_APP, } from '../../stores/events/PresenceStore.js'; -import type { - DurableSubscription, - SubscriptionTarget, -} from '../../stores/events/types.js'; +import type { DurableSubscription } from '../../stores/events/types.js'; import type { FSEntry } from '../../stores/fs/FSEntry.js'; import type { IConfig } from '../../types.js'; import { EventForwardService } from './EventForwardService.js'; import { EventsService, type DeliveryEnvelope } from './EventsService.js'; +import { appSocketRoom } from '../socket/SocketService.js'; import type { ForwardBatch, ForwardDelivery, @@ -237,6 +235,9 @@ const entry = (over: Partial = {}): FSEntry => ...over, }) as FSEntry; +/** An app whose rows run a handler in its events worker. */ +const APP_UID = 'app-forwarded'; + const actorFor = ( appUid: string | null = null, holderUserId: number = userId, @@ -267,7 +268,6 @@ const durableRow = ( appUid: null, permission: 'list', delivery: 'broadcast', - targets: ['socket'] as SubscriptionTarget[], handlerName: null, context: null, expiresAt: null, @@ -453,7 +453,6 @@ const makeRegion = ( op: (input.op as DurableSubscription['op']) ?? null, appUid: (input.appUid as string | null) ?? null, delivery: input.delivery as DurableSubscription['delivery'], - targets: input.targets as DurableSubscription['targets'], handlerName: (input.handlerName as string | null) ?? null, context: (input.context as string | null) ?? null, permission: @@ -1062,23 +1061,20 @@ describe('a delivery the worker takes', () => { }); }); - it('fans the mark out with a broadcast the worker takes', async () => { + it('fans an unmarked copy out while the worker also runs it', async () => { const west = makeRegion('west', ['east']); const east = makeRegion('east', ['west']); - east.rooms.add(String(userId)); - await east.forward.noteConnect(actorFor(), 'tab-1'); - await register(west, { - targets: ['socket', 'worker'] as SubscriptionTarget[], - handlerName: 'onWrite', - }); + east.rooms.add(appSocketRoom(userId, APP_UID)); + await east.forward.noteConnect(actorFor(APP_UID), 'tab-1'); + await register(west, { appUid: APP_UID, handlerName: 'onWrite' }); await dispatch(west); await posted(west); await arrived(east); expect(west.invoked).toHaveLength(1); - expect(deliveriesIn(west)[0]).toMatchObject({ skipHandler: true }); - expect(east.sent[0]).toMatchObject({ skipHandler: true }); + expect(deliveriesIn(west)[0].skipHandler).toBeUndefined(); + expect(east.sent[0].skipHandler).toBeUndefined(); }); }); @@ -1350,7 +1346,6 @@ describe('a delivery owed to exactly one consumer', () => { const single = (region: Region) => register(region, { delivery: 'single', - targets: ['socket', 'worker'] as SubscriptionTarget[], handlerName: 'onWrite', }); @@ -1389,33 +1384,6 @@ describe('a delivery owed to exactly one consumer', () => { expect(eastA.invoked).toEqual([]); }); - it('moves on to the next region when the one it tried leaves the row', async () => { - const west = makeRegion('west', ['east', 'south']); - const east = makeRegion('east', ['west', 'south']); - const south = makeRegion('south', ['west', 'east']); - east.rooms.add(String(userId)); - south.rooms.add(String(userId)); - await east.forward.noteConnect(actorFor(), 'tab-1'); - await quiet(5); - await south.forward.noteConnect(actorFor(), 'tab-1'); - await single(west); - - await dispatch(west); - await arrived(south); - - // South's client goes without acking, and south leaves the row. - south.rooms.clear(); - await south.forward.noteDisconnect(actorFor(), 'tab-1'); - await vi.waitFor(() => expect(rowFor().regions.south).toBeUndefined()); - - jump(61_000); - await west.events.sweepPending(); - await arrived(east); - - expect(east.sent[0]).toMatchObject({ ackRequired: true }); - expect(west.invoked).toEqual([]); - }); - it('hands off to the most recently connected region when it holds none', async () => { const west = makeRegion('west', ['east', 'south']); const east = makeRegion('east', ['west', 'south']); @@ -1492,6 +1460,43 @@ describe('a delivery owed to exactly one consumer', () => { }); }); + it('sends a decline home, and the emitting region runs the worker', async () => { + const west = makeRegion('west', ['east']); + const east = makeRegion('east', ['west']); + east.rooms.add(appSocketRoom(userId, APP_UID)); + await east.forward.noteConnect(actorFor(APP_UID), 'tab-1'); + workerOutcome = 'settled'; + const row = await register(west, { + appUid: APP_UID, + delivery: 'single', + handlerName: 'onWrite', + }); + + await dispatch(west); + await posted(west); + await arrived(east); + expect(west.invoked).toEqual([]); + + const envelope = east.sent[0]; + await east.events.declineDelivery(actorFor(APP_UID), { + subId: row.subId, + id: envelope.ackId, + origin: envelope.origin, + reason: 'no_handler', + }); + + await vi.waitFor(() => expect(west.invoked).toHaveLength(1), { + timeout: 3_000, + interval: 20, + }); + expect(east.invoked).toEqual([]); + expect(east.posts.at(-1)?.batch.items[0]).toMatchObject({ + kind: 'nack', + subId: row.subId, + reason: 'no_handler', + }); + }); + it('never settles a lease in the region the client happened to reach', async () => { const west = makeRegion('west', ['east']); const east = makeRegion('east', ['west']); @@ -1512,13 +1517,17 @@ describe('a delivery owed to exactly one consumer', () => { expect(await east.pending.depth(row.subId)).toBe(0); }); - it('gives the handler what two socket candidates could not take', async () => { + it('gives the handler what the socket candidate could not take', async () => { const west = makeRegion('west', ['east']); const east = makeRegion('east', ['west']); - east.rooms.add(String(userId)); - await east.forward.noteConnect(actorFor(), 'tab-1'); + east.rooms.add(appSocketRoom(userId, APP_UID)); + await east.forward.noteConnect(actorFor(APP_UID), 'tab-1'); workerOutcome = 'settled'; - const row = await single(west); + const row = await register(west, { + appUid: APP_UID, + delivery: 'single', + handlerName: 'onWrite', + }); await dispatch(west); await posted(west); @@ -1862,7 +1871,6 @@ describe('forward-path metrics', () => { await south.forward.noteConnect(actorFor(), 'tab-1'); await register(west, { delivery: 'single', - targets: ['socket', 'worker'] as SubscriptionTarget[], handlerName: 'onWrite', }); @@ -1894,7 +1902,6 @@ describe('forward-path metrics', () => { await east.forward.noteConnect(actorFor(), 'tab-1'); await register(west, { delivery: 'single', - targets: ['socket', 'worker'] as SubscriptionTarget[], handlerName: 'onWrite', }); @@ -1911,12 +1918,12 @@ describe('forward-path metrics', () => { it('counts a single attempt that reaches the handler', async () => { const west = makeRegion('west', ['east']); const east = makeRegion('east', ['west']); - east.rooms.add(String(userId)); - await east.forward.noteConnect(actorFor(), 'tab-1'); + east.rooms.add(appSocketRoom(userId, APP_UID)); + await east.forward.noteConnect(actorFor(APP_UID), 'tab-1'); workerOutcome = 'settled'; await register(west, { + appUid: APP_UID, delivery: 'single', - targets: ['socket', 'worker'] as SubscriptionTarget[], handlerName: 'onWrite', }); @@ -2332,7 +2339,6 @@ describe('a durable row created in another region', () => { await east.events.subscribeDurable(actorFor(), { subject: `fs:${anchorUid()}`, - targets: ['socket'], }); // A tick for the 25 ms addressed queue — the bus stays held. await quiet(80); @@ -2354,7 +2360,6 @@ describe('a durable row created in another region', () => { await east.events.subscribeDurable(actorFor(), { subject: `fs:${anchorUid()}`, - targets: ['socket'], }); await quiet(80); diff --git a/src/backend/services/events/handlers.integration.test.ts b/src/backend/services/events/handlers.integration.test.ts index 49adfbcc6..600a3ca93 100644 --- a/src/backend/services/events/handlers.integration.test.ts +++ b/src/backend/services/events/handlers.integration.test.ts @@ -700,7 +700,6 @@ describe('binding a subscription to a handler', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], }); expect(created.status).toBe(200); @@ -712,7 +711,6 @@ describe('binding a subscription to a handler', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], }); expect(refused.status).toBe(404); @@ -728,7 +726,6 @@ describe('binding a subscription to a handler', () => { delivery: 'single', handlerName: 'ingestUpload', handlerHash: hash, - targets: ['worker'], }); expect(created.status).toBe(200); @@ -742,7 +739,6 @@ describe('binding a subscription to a handler', () => { delivery: 'single', handlerName: 'ingestUpload', handlerHash: hashContent(NEXT_SOURCE), - targets: ['worker'], }); expect(refused.status).toBe(409); @@ -754,7 +750,6 @@ describe('binding a subscription to a handler', () => { const refused = await subscribe(appToken, { subject: `fs:${anchor}`, delivery: 'single', - targets: ['worker'], }); expect(refused.status).toBe(400); @@ -769,7 +764,6 @@ describe('the handler lifecycle', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], }); expect(created.status).toBe(200); return String(created.body.subId); @@ -1070,7 +1064,6 @@ describe('the context a subscription carries', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], context: { url: 'https://ingest.example/secret-token', retries: 3 }, }); expect(created.status).toBe(200); @@ -1087,7 +1080,6 @@ describe('the context a subscription carries', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], }); const [listed] = await listSubscriptions(appToken); @@ -1101,7 +1093,6 @@ describe('the context a subscription carries', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], context: { blob: 'x'.repeat(5000) }, }); @@ -1116,14 +1107,12 @@ describe('the context a subscription carries', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], context: { url: 'https://mine.example' }, }); const theirs = await subscribe(otherAppToken, { subject: `fs:${otherAnchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], context: { url: 'https://theirs.example' }, }); expect(mine.status).toBe(200); diff --git a/src/backend/services/events/metering.test.ts b/src/backend/services/events/metering.test.ts index 03f1a830f..56d61c239 100644 --- a/src/backend/services/events/metering.test.ts +++ b/src/backend/services/events/metering.test.ts @@ -82,7 +82,6 @@ const durableRow = ( appUid: null, permission: 'list', delivery: 'broadcast', - targets: ['socket'], handlerName: null, context: null, expiresAt: null, @@ -151,7 +150,6 @@ beforeEach(() => { const row = durableRow({ appUid: input.appUid, delivery: input.delivery, - targets: input.targets, }); rows.push(row); return { diff --git a/src/backend/services/events/revocationSettle.integration.test.ts b/src/backend/services/events/revocationSettle.integration.test.ts index 89e3a3a11..7dcf825f4 100644 --- a/src/backend/services/events/revocationSettle.integration.test.ts +++ b/src/backend/services/events/revocationSettle.integration.test.ts @@ -198,13 +198,18 @@ const makeApp = async (path: string): Promise => { uid, `fs:${await uidOf(path)}:list`, ); - // Durable rows target the app's worker by default, which takes its own + // A row with a handler runs in the app's worker, which takes its own // consent. await env.server.services.permission.grantUserAppPermission( owner.actor, uid, EVENTS_BACKGROUND_PERMISSION, ); + await env.server.stores.eventHandler.publish({ + appUid: uid, + name: 'onChange', + source: 'async () => {}', + }); const app = await env.server.stores.app.getByUid(uid); return makeActor({ user: owner.actor.user as never, @@ -698,14 +703,16 @@ describe('what a revoked grant settles', () => { const appActor = await makeApp(path); const background = ( - await events().subscribeDurable(appActor, { subject: `fs:${path}` }) + await events().subscribeDurable(appActor, { + subject: `fs:${path}`, + handlerName: 'onChange', + }) ).sub; - // Delivered to a connection and nowhere else, so the consent that just - // went was never what allowed it. + // No handler, so it only reaches connections and the consent that + // just went was never what allowed it. const foreground = ( await events().subscribeDurable(appActor, { subject: `fs:${path}`, - targets: ['socket'], }) ).sub; @@ -729,6 +736,7 @@ describe('what a revoked grant settles', () => { ( await events().subscribeDurable(appActor, { subject: `fs:${path}`, + handlerName: 'onChange', }) ).sub, ); diff --git a/src/backend/services/events/singleDelivery.test.ts b/src/backend/services/events/singleDelivery.test.ts index 019c2b015..40b6e9a62 100644 --- a/src/backend/services/events/singleDelivery.test.ts +++ b/src/backend/services/events/singleDelivery.test.ts @@ -23,16 +23,14 @@ import { EVENTS_COALESCE_WINDOW_MS, EVENTS_REGION_PENDING_CEILING, EVENTS_SINGLE_DELIVERY_LIMIT, + EVENTS_SOCKET_LEASE_MS, EVENTS_WORKER_INVOCATION_LIMIT, deliveryBackoffMs, } from '../../controllers/events/limits.js'; import type { Actor } from '../../core/actor.js'; import { EventSubscriptionStore } from '../../stores/events/EventSubscriptionStore.js'; import { PendingDeliveryStore } from '../../stores/events/PendingDeliveryStore.js'; -import type { - DurableSubscription, - SubscriptionTarget, -} from '../../stores/events/types.js'; +import type { DurableSubscription } from '../../stores/events/types.js'; import type { FSEntry } from '../../stores/fs/FSEntry.js'; import type { IConfig } from '../../types.js'; import type { UsageInput } from '../metering/types.js'; @@ -133,10 +131,9 @@ const durableRow = ( anchorPath: anchorPath(), match: null, op: null, - appUid: null, + appUid: `app-${seq}`, permission: 'list', delivery: 'single', - targets: ['socket', 'worker'] as SubscriptionTarget[], handlerName: 'onWrite', context: null, expiresAt: null, @@ -278,6 +275,7 @@ beforeEach(async () => { fanOut: async () => undefined, handOff: () => undefined, relayAck: () => undefined, + relayNack: () => undefined, announceWatch: () => undefined, forwardEvent: () => undefined, announceGeneration: () => undefined, @@ -402,37 +400,52 @@ describe('a delivery owed to exactly one consumer', () => { expect(sent[1].event.id).not.toBe(sent[0].event.id); }); - it('tries the socket twice, then hands it to the handler', async () => { + it('tries the socket once, then hands it to the handler', async () => { const row = await register(); await dispatch(); expect(sent).toHaveLength(1); - // First lease lapses with no ack: a second socket attempt, which is - // what a reconnected client is worth. - jump(61_000); + // Still within the socket lease: nothing moves. + jump(EVENTS_SOCKET_LEASE_MS - 1_000); await service.sweepPending(); - expect(sent).toHaveLength(2); expect(invoked).toEqual([]); - // Second lapses too. Sockets are spent, so the handler takes it. + // The client never answered. Its one attempt is spent, so the handler + // takes it. workerOutcome = 'settled'; - jump(61_000); + jump(2_000); await service.sweepPending(); - expect(sent).toHaveLength(2); + expect(sent).toHaveLength(1); expect(invoked).toHaveLength(1); expect(invoked[0]).toMatchObject({ subId: row.subId, handlerName: 'onWrite', holderUserId: userId, }); - // Three attempts at the one event it was never acked for — not three + // Two attempts at the one event it was never acked for — not two // bills. A retry after a lease expiry is not a new delivery. expect(metered).toHaveLength(1); }); + it('holds the worker to its own, longer lease', async () => { + socketConnected = false; + await register(); + await dispatch(); + expect(invoked).toHaveLength(1); + + // Past a socket lease, but not the worker's: no second run yet. + jump(EVENTS_SOCKET_LEASE_MS + 1_000); + await service.sweepPending(); + expect(invoked).toHaveLength(1); + + jump(61_000); + await service.sweepPending(); + expect(invoked).toHaveLength(2); + }); + it('bills a retried delivery once, however many attempts an outage costs', async () => { socketConnected = false; - const row = await register({ targets: ['worker'] }); + const row = await register(); await dispatch(); expect(invoked).toHaveLength(1); @@ -458,10 +471,10 @@ describe('a delivery owed to exactly one consumer', () => { await expect(pending.depth(row.subId)).resolves.toBe(0); }); - it('does not bill or report a retriable or terminal handler outcome', async () => { + it('bills nothing for a retriable outcome, and once for a dropped one', async () => { socketConnected = false; workerOutcome = 'retriable'; - const row = await register({ targets: ['worker'] }); + const row = await register(); await dispatch(); @@ -474,18 +487,20 @@ describe('a delivery owed to exactly one consumer', () => { jump(deliveryBackoffMs(1) + 1); await service.sweepPending(); - // Discarded with a gap marker rather than delivered — still no bill. - expect(metered).toEqual([]); - expect(delivered).toEqual([]); + // Dropped with a gap marker in its place. A run was made, so it is + // billed like a completed one. + expect(metered).toHaveLength(1); + expect(delivered).toHaveLength(1); + await expect(redis.get(`ev:qf:{${row.subId}}`)).resolves.toBe('2'); }); it('resumes a socket-only account row once its client reconnects, rather than wedging forever', async () => { - const row = await register({ appUid: null, targets: ['socket'] }); + const row = await register({ appUid: null }); await dispatch(); expect(sent).toHaveLength(1); - // A second attempt while still connected spends the whole budget — - // there is no worker to fall back to on this row. + // Still connected once the lease lapses: with no worker to fall back + // to, the client gets another attempt. jump(61_000); await service.sweepPending(); expect(sent).toHaveLength(2); @@ -510,26 +525,149 @@ describe('a delivery owed to exactly one consumer', () => { await expect(pending.depth(row.subId)).resolves.toBe(0); }); - it('does not skip a remote candidate when the local socket disappears between attempts', async () => { - const regions = ['east', 'west']; - candidateRegionImpl = async (_holderUserId, _appUid, tried) => - regions.find((region) => !tried.includes(region)) ?? null; - await register({ targets: ['socket', 'worker'] }); + it('offers it to a region presence names when nothing is connected here', async () => { + socketConnected = false; + candidateRegionImpl = async () => 'east'; + await register(); await dispatch(); - // The local socket takes the first attempt; no remote candidate is - // spent on a first attempt that had somewhere to go. - expect(sent).toHaveLength(1); - expect(candidateRegionCalls).toEqual([]); - // The local socket is gone by the next attempt. - socketConnected = false; + expect(candidateRegionCalls).toEqual([[]]); + expect(sent).toEqual([]); + expect(invoked).toEqual([]); + }); + + it('hands a delivery the client has no handler for straight to the worker', async () => { + workerOutcome = 'settled'; + const row = await register(); + await dispatch(); + expect(sent).toHaveLength(1); + + await service.declineDelivery(actorFor(), { + subId: row.subId, + id: sent[0].ackId, + reason: 'no_handler', + }); + + expect(invoked).toHaveLength(1); + await expect(pending.depth(row.subId)).resolves.toBe(0); + // Not a failure: nothing ran. + await expect(redis.get(`ev:qf:{${row.subId}}`)).resolves.toBeNull(); + expect(metered).toHaveLength(1); + }); + + it('counts a retryable client failure, and retries in the worker after the backoff', async () => { + workerOutcome = 'settled'; + const row = await register(); + await dispatch(); + + await service.declineDelivery(actorFor(), { + subId: row.subId, + id: sent[0].ackId, + reason: 'failed', + retry: true, + }); + + expect(invoked).toEqual([]); + await expect(redis.get(`ev:qf:{${row.subId}}`)).resolves.toBe('1'); + + // Held for the backoff, not offered again before it. + await service.sweepPending(); + expect(invoked).toEqual([]); + + jump(deliveryBackoffMs(1) + 1); + await service.sweepPending(); + expect(sent).toHaveLength(1); + expect(invoked).toHaveLength(1); + await expect(pending.depth(row.subId)).resolves.toBe(0); + }); + + it('drops a client failure that is not retryable, and counts it', async () => { + const row = await register(); + await dispatch(); + + await service.declineDelivery(actorFor(), { + subId: row.subId, + id: sent[0].ackId, + reason: 'failed', + retry: false, + }); + + expect(invoked).toEqual([]); + await expect(redis.get(`ev:qf:{${row.subId}}`)).resolves.toBe('1'); + // Billed once, on the attempt that reached the client. + expect(metered).toHaveLength(1); + + // The event itself is gone; what follows is the gap marker in its place. + await service.sweepPending(); + const after = sent.slice(1).map((envelope) => envelope.event); + expect(after.every((event) => event.op === 'gap')).toBe(true); + }); + + it('pauses the subscription after five failures in a row', async () => { + const row = await register(); + await dispatch(); + + for (let i = 0; i < 5; i++) { + const last = sent.at(-1)!; + await service.declineDelivery(actorFor(), { + subId: row.subId, + id: last.ackId, + reason: 'failed', + retry: false, + }); + // The next delivery out is the marker the drop left behind. + if (i < 4) await service.sweepPending(); + } + + expect(rows.get(row.subId)).toMatchObject({ + suspendedReason: 'failures', + }); + }); + + it('ignores a duplicate or stale decline', async () => { + workerOutcome = 'deferred'; + const row = await register(); + await dispatch(); + const decline = { + subId: row.subId, + id: sent[0].ackId, + reason: 'no_handler', + }; + + await service.declineDelivery(actorFor(), decline); + await service.declineDelivery(actorFor(), decline); + expect(invoked).toHaveLength(1); + + // The worker's lease lapses and it runs again; a decline arriving now + // names a lease that is no longer the client's. jump(61_000); await service.sweepPending(); + expect(invoked).toHaveLength(2); + await service.declineDelivery(actorFor(), decline); + expect(invoked).toHaveLength(2); - // The first remote candidate, not the second — nothing was actually - // spent on a remote region before this. - expect(candidateRegionCalls).toEqual([[]]); + // And one for a delivery already settled changes nothing. + workerOutcome = 'settled'; + jump(61_000); + await service.sweepPending(); + expect(invoked).toHaveLength(3); + await service.declineDelivery(actorFor(), decline); + expect(invoked).toHaveLength(3); + }); + + it('leaves a declined delivery to its lease on a row with no worker', async () => { + const row = await register({ appUid: null }); + await dispatch(); + + await service.declineDelivery(actorFor(), { + subId: row.subId, + id: sent[0].ackId, + reason: 'no_handler', + }); + + expect(invoked).toEqual([]); + await expect(pending.depth(row.subId)).resolves.toBe(1); }); it('goes straight to the handler when nothing is connected', async () => { @@ -623,14 +761,15 @@ describe('a delivery owed to exactly one consumer', () => { }); describe('a delivery everyone connected gets', () => { - it('hands the run to the worker and marks the socket copy', async () => { + it('runs the worker and sends an unmarked copy to the sockets', async () => { const row = await register({ delivery: 'broadcast' }); await dispatch(); await flushed(); expect(sent).toHaveLength(1); - expect(sent[0]).toMatchObject({ subId: row.subId, skipHandler: true }); + expect(sent[0]).toMatchObject({ subId: row.subId }); + expect(sent[0].skipHandler).toBeUndefined(); expect(sent[0].ackRequired).toBeUndefined(); expect(invoked).toHaveLength(1); expect(invoked[0]).toMatchObject({ subId: row.subId }); @@ -638,7 +777,37 @@ describe('a delivery everyone connected gets', () => { expect(delivered).toHaveLength(1); }); - it('leaves the socket copy unmarked, and never invokes the worker, when it has no runtime', async () => { + it('still runs the worker with no socket connected', async () => { + socketConnected = false; + workerOutcome = 'settled'; + await register({ delivery: 'broadcast' }); + + await dispatch(); + await vi.waitFor(() => expect(invoked.length).toBe(1), { + timeout: EVENTS_COALESCE_WINDOW_MS * 12, + interval: 25, + }); + + expect(delivered).toHaveLength(1); + expect(metered).toHaveLength(1); + }); + + it('invokes no worker for a row with no app or no handler', async () => { + await register({ delivery: 'broadcast', appUid: null }); + await register({ + subId: 'unnamed', + delivery: 'broadcast', + handlerName: null, + }); + + await dispatch(); + await flushed(2); + + expect(sent).toHaveLength(2); + expect(invoked).toEqual([]); + }); + + it('never invokes the worker when it has no runtime', async () => { service.worker = { invoke: async (invocation: WorkerInvocation) => { invoked.push(invocation); @@ -653,11 +822,10 @@ describe('a delivery everyone connected gets', () => { expect(sent).toHaveLength(1); expect(sent[0]).toMatchObject({ subId: row.subId }); - expect(sent[0].skipHandler).toBeUndefined(); expect(invoked).toEqual([]); }); - it("leaves the socket copy unmarked for whichever delivery lands once the app's invocations for the minute are spent", async () => { + it("keeps sending to sockets once the app's invocations for the minute are spent", async () => { const appUid = `app-${seq}`; await register({ delivery: 'broadcast', appUid }); const total = EVENTS_WORKER_INVOCATION_LIMIT.limit + 1; @@ -674,39 +842,23 @@ describe('a delivery everyone connected gets', () => { await flushed(total); expect(invoked).toHaveLength(EVENTS_WORKER_INVOCATION_LIMIT.limit); + expect(sent).toHaveLength(total); expect( sent.filter((envelope) => envelope.skipHandler === true), - ).toHaveLength(EVENTS_WORKER_INVOCATION_LIMIT.limit); - expect( - sent.filter((envelope) => envelope.skipHandler === undefined), - ).toHaveLength(1); + ).toEqual([]); }); - it('runs the handler once per delivery, not once per transport', async () => { - workerOutcome = 'settled'; - await register({ delivery: 'broadcast', targets: ['worker'] }); + it('delivers to every transport for a row stored with old targets', async () => { + await register({ + delivery: 'broadcast', + targets: ['push'], + } as Partial); await dispatch(); - await vi.waitFor(() => expect(invoked.length).toBe(1), { - timeout: EVENTS_COALESCE_WINDOW_MS * 12, - interval: 25, - }); + await flushed(); - expect(sent).toEqual([]); - expect(delivered).toHaveLength(1); - }); - - it('delivers and meters nothing when nothing can carry it', async () => { - await register({ delivery: 'broadcast', targets: ['push'] }); - - await dispatch(); - await new Promise((resolve) => - setTimeout(resolve, EVENTS_COALESCE_WINDOW_MS * 3), - ); - - expect(sent).toEqual([]); - expect(invoked).toEqual([]); - expect(delivered).toEqual([]); + expect(sent).toHaveLength(1); + expect(invoked).toHaveLength(1); }); }); @@ -789,6 +941,7 @@ describe('keeping the region counter honest', () => { describe('the sweeper does not let one stuck backlog starve another', () => { it('still sweeps a fresh delivery behind 100+ blocked subscriptions', async () => { + socketConnected = false; workerOutcome = 'settled'; const rawEvent = (id: string) => ({ id, @@ -802,17 +955,14 @@ describe('the sweeper does not let one stuck backlog starve another', () => { }); for (let i = 0; i < 100; i++) { - const row = await register({ - subId: `blocked-${i}`, - targets: ['worker'], - }); + const row = await register({ subId: `blocked-${i}` }); await pending.enqueue(row.subId, rawEvent(`blocked-${i}`)); // Already in flight for the whole pass — a subscription in retry // backoff answers `claim()` the same way. await pending.claim(row.subId, { leaseMs: 10 * 60_000 }); } - const fresh = await register({ subId: 'fresh', targets: ['worker'] }); + const fresh = await register({ subId: 'fresh' }); await pending.enqueue(fresh.subId, rawEvent('fresh')); // One pass only ever reaches the 100 blocked subscriptions ahead of @@ -828,8 +978,9 @@ describe('the sweeper does not let one stuck backlog starve another', () => { describe('the sweep order a busy subscription holds', () => { it('is untouched by a publish, and only moved by a sweep pass', async () => { + socketConnected = false; workerOutcome = 'deferred'; - const row = await register({ targets: ['worker'] }); + const row = await register(); await dispatch(entry({ uid: `file-a-${seq}` })); // Held in flight: the worker never settled it, so the lease stands. @@ -915,7 +1066,7 @@ describe('what a delivery costs its holder', () => { describe('the budgets a `single` is delivered under', () => { it('stands a gap marker in for an event past the per-minute budget', async () => { - const row = await register({ targets: ['socket'] }); + const row = await register({ appUid: null }); // One consumer holds one lease at a time, so this is the client loop: // take the delivery, ack it, and let the next one out. @@ -945,7 +1096,7 @@ describe('the budgets a `single` is delivered under', () => { socketConnected = false; workerOutcome = 'settled'; const appUid = `app-${seq}`; - const row = await register({ appUid, targets: ['worker'] }); + const row = await register({ appUid }); for (let i = 0; i <= EVENTS_WORKER_INVOCATION_LIMIT.limit; i++) await dispatch(entry({ uid: `file-${seq}-${i}` })); diff --git a/src/backend/services/events/workerInvoker.integration.test.ts b/src/backend/services/events/workerInvoker.integration.test.ts index b35cc85c3..81a984e05 100644 --- a/src/backend/services/events/workerInvoker.integration.test.ts +++ b/src/backend/services/events/workerInvoker.integration.test.ts @@ -160,7 +160,6 @@ const subscribe = async (token: string = appToken): Promise => { subject: `fs:${anchor}`, delivery: 'single', handlerName: HANDLER, - targets: ['worker'], context: { label: 'ingest' }, }), }); @@ -169,7 +168,7 @@ const subscribe = async (token: string = appToken): Promise => { return body.subId; }; -/** A `broadcast` subscription, left at its default targets (socket + worker). */ +/** A `broadcast` subscription with a handler. */ const subscribeBroadcast = async ( token: string = appToken, ): Promise => { @@ -258,7 +257,6 @@ const subscribeKv = async ( subject: `kv:${key}`, delivery: 'single', handlerName: HANDLER, - targets: ['worker'], }), }); const body = (await response.json()) as { subId: string }; @@ -723,8 +721,8 @@ describe('what each answer does to the delivery', () => { }); }); -describe('a broadcast delivery, with a worker to take it', () => { - it('hands the run to the worker and marks the socket copy', async () => { +describe('a broadcast delivery with a handler', () => { + it('runs the worker and sends the clients an unmarked copy', async () => { const send = vi.spyOn(env.server.services.socket, 'send'); try { await subscribeBroadcast(); @@ -733,11 +731,14 @@ describe('a broadcast delivery, with a worker to take it', () => { await invoked(1); expect(calls).toHaveLength(1); - expect(send).toHaveBeenCalledWith( - expect.anything(), - 'events.delivery', - expect.objectContaining({ skipHandler: true }), + const copies = send.mock.calls.filter( + ([, channel]) => channel === 'events.delivery', ); + expect(copies.length).toBeGreaterThan(0); + for (const [, , envelope] of copies) + expect( + (envelope as { skipHandler?: true }).skipHandler, + ).toBeUndefined(); } finally { send.mockRestore(); } diff --git a/src/backend/services/events/workerRuntime.integration.test.ts b/src/backend/services/events/workerRuntime.integration.test.ts index ad41d77ed..d0c237aee 100644 --- a/src/backend/services/events/workerRuntime.integration.test.ts +++ b/src/backend/services/events/workerRuntime.integration.test.ts @@ -284,28 +284,6 @@ const rehydrate = async ( const subscribe = async ( handlerName: string, context?: object, -): Promise => { - const response = await api('POST', '/events/subscribe', { - subject: `fs:${anchor}`, - delivery: 'single', - handlerName, - targets: ['worker'], - ...(context ? { context } : {}), - }); - expect(response.status).toBe(200); - return String(response.body.subId); -}; - -/** - * Like `subscribe`, but with no `targets` override — the default - * (`['socket', 'worker']`) rather than the `['worker']` the rest of this suite - * pins. Only this leaves `socket` in play, which is what a background - * delivery's own client (wrongly) showing up in the app's delivery room would - * actually steer onto. - */ -const subscribeWithSocketTarget = async ( - handlerName: string, - context?: object, ): Promise => { const response = await api('POST', '/events/subscribe', { subject: `fs:${anchor}`, @@ -676,10 +654,7 @@ describe('the client a handler runs as', () => { { timeout: TEST_TIMEOUT_MS }, async () => { const room = appSocketRoom(userId, appUid); - // `subscribeWithSocketTarget` leaves `socket` a live option for - // this delivery — the ordinary `subscribe()` helper pins - // `targets: ['worker']` specifically to avoid exercising this. - await subscribeWithSocketTarget('ingest', { sink: sinkUrl }); + await subscribe('ingest', { sink: sinkUrl }); await touch('first.txt'); await delivered(1); diff --git a/src/backend/services/events/workerSource.test.ts b/src/backend/services/events/workerSource.test.ts index 51a9f0373..996f615b9 100644 --- a/src/backend/services/events/workerSource.test.ts +++ b/src/backend/services/events/workerSource.test.ts @@ -48,7 +48,16 @@ const RUNTIME_PATH = path.join( path.dirname(fileURLToPath(import.meta.url)), '../../../worker/src/events-runtime.js', ); -const RUNTIME_SOURCE = readFileSync(RUNTIME_PATH, 'utf8'); +// What the preamble build inlines ahead of the runtime: the shared retry +// classifier, without its `export`. +const RETRYABLE_SOURCE = readFileSync( + path.join( + path.dirname(fileURLToPath(import.meta.url)), + '../../../puter-js/src/modules/events/lib/retryable.js', + ), + 'utf8', +).replace(/^export (const|function) /gm, '$1 '); +const RUNTIME_SOURCE = RETRYABLE_SOURCE + readFileSync(RUNTIME_PATH, 'utf8'); const INVOKE_KEY = 'k1:test-key'; const INVOKE_URL = `http://evw-test.example${EVENTS_INVOKE_PATH}`; @@ -282,7 +291,7 @@ describe('the generated worker inside its runtime', () => { expect(Object.isFrozen(seen.mock.calls[0][0])).toBe(true); }); - it('takes resolution as the ack, and a throw as retriable', async () => { + it('takes resolution as the ack, and a plain throw as a refusal', async () => { const worker = loadWorker([ handler('ok', 'async () => {}'), handler('boom', 'async () => { throw new Error("kaput"); }'), @@ -295,7 +304,7 @@ describe('the generated worker inside its runtime', () => { const failed = await invoke(worker, { body: delivery({ handler: 'boom' }), }); - expect(failed.status).toBe(500); + expect(failed.status).toBe(400); expect(await failed.json()).toEqual({ error: 'kaput' }); }); @@ -339,6 +348,32 @@ describe('the generated worker inside its runtime', () => { ).toBe(400); }); + it('answers a retryable failure with a retry, and any other throw with a refusal', async () => { + const worker = loadWorker([ + handler('plain', 'async () => { throw new Error("bad input"); }'), + handler( + 'marked', + 'async () => { throw Object.assign(new Error("later"), { retry: true }); }', + ), + handler( + 'busy', + 'async () => { throw Object.assign(new Error("busy"), { status: 503 }); }', + ), + handler( + 'offline', + 'async () => { throw new TypeError("fetch failed"); }', + ), + ]); + const statusOf = async (name: string) => + (await invoke(worker, { body: delivery({ handler: name }) })) + .status; + + expect(await statusOf('plain')).toBe(400); + expect(await statusOf('marked')).toBe(500); + expect(await statusOf('busy')).toBe(500); + expect(await statusOf('offline')).toBe(500); + }); + it('isolates a handler whose source does not parse', async () => { const worker = loadWorker([ handler('good', SINK), @@ -402,6 +437,10 @@ describe('the handled marker', () => { throw err; }`), handler('boom', 'async () => { throw new Error("x"); }'), + handler( + 'later', + 'async () => { throw Object.assign(new Error("x"), { retry: true }); }', + ), handler('bad', 'async ( => {'), ]); @@ -411,7 +450,8 @@ describe('the handled marker', () => { [{ body: delivery({ handler: 'bad' }) }, 'handler-broken'], [{ body: delivery({ handler: 'missing' }) }, 'unknown-handler'], [{ body: { handler: 'ok' } }, 'no-token'], - [{ body: delivery({ handler: 'boom' }) }, 'handler-threw'], + [{ body: delivery({ handler: 'later' }) }, 'handler-threw'], + [{ body: delivery({ handler: 'boom' }) }, 'handler-terminal'], [{ body: delivery({ handler: 'flagged' }) }, 'handler-terminal'], ]; for (const [options, code] of cases) diff --git a/src/backend/services/events/workers.integration.test.ts b/src/backend/services/events/workers.integration.test.ts index 397a85a13..9b3e1f09c 100644 --- a/src/backend/services/events/workers.integration.test.ts +++ b/src/backend/services/events/workers.integration.test.ts @@ -166,7 +166,6 @@ const subscribeBackgroundHandler = async ( subject: `fs:${anchor}`, delivery: 'single', handlerName, - targets: ['worker'], }); expect(subscribed.status).toBe(200); }; @@ -473,7 +472,6 @@ describe('POST /events/workers/destroy', () => { subject: `fs:${anchor}`, delivery: 'single', handlerName: 'ingestUpload', - targets: ['worker'], }); expect(subscribed.status).toBe(200); diff --git a/src/backend/stores/events/DurableSubscriptionStore.integration.test.ts b/src/backend/stores/events/DurableSubscriptionStore.integration.test.ts index 07e71b2d3..7bd8aef8c 100644 --- a/src/backend/stores/events/DurableSubscriptionStore.integration.test.ts +++ b/src/backend/stores/events/DurableSubscriptionStore.integration.test.ts @@ -67,11 +67,6 @@ const input = ( match: null, op: null, delivery: 'broadcast', - // Null `appUid` is the default here (a session/account row), and a - // worker target needs an app to run one for — see "one events worker per - // app" below. Tests that want an app-scoped row with a worker target - // override both. - targets: ['socket'], handlerName: null, context: null, permission: 'list', @@ -128,7 +123,6 @@ describe('creating a subscription', () => { appUid, handlerName: 'onWrite', match: '*.txt', - targets: ['socket', 'worker'], }), ); @@ -139,11 +133,47 @@ describe('creating a subscription', () => { handlerName: 'onWrite', match: '*.txt', delivery: 'broadcast', - targets: ['socket', 'worker'], durable: true, }); }); + it('writes the legacy targets column and never reads it back', async () => { + const appUid = `app-${uuidv4()}`; + const { row: appRow } = await durable().create(input({ appUid })); + const { row: accountRow } = await durable().create(input()); + + const stored = async (subId: string) => { + const [raw] = await env.server.clients.db.read( + 'SELECT `targets` FROM `event_subscriptions` WHERE `sub_id` = ?', + [subId], + ); + const value = (raw as { targets: unknown }).targets; + return typeof value === 'string' ? JSON.parse(value) : value; + }; + await expect(stored(appRow.subId)).resolves.toEqual([ + 'socket', + 'worker', + ]); + await expect(stored(accountRow.subId)).resolves.toEqual(['socket']); + expect( + (await durable().getBySubId(appRow.subId)) as unknown as Record< + string, + unknown + >, + ).not.toHaveProperty('targets'); + }); + + it('reads any stored delivery other than `single` as a broadcast', async () => { + const { row } = await durable().create(input()); + await env.server.clients.db.write( + 'UPDATE `event_subscriptions` SET `delivery` = ? WHERE `sub_id` = ?', + ['fanout', row.subId], + ); + await expect(durable().getBySubId(row.subId)).resolves.toMatchObject({ + delivery: 'broadcast', + }); + }); + it('remembers whether deliveries carry the value, off by default', async () => { const { row: asking } = await durable().create( input({ includeValue: true }), @@ -232,87 +262,6 @@ describe('creating a subscription', () => { }); describe('validation at the row write', () => { - it('refuses a target outside the known set', async () => { - await expect( - durable().create( - input({ targets: ['socket', 'carrier-pigeon'] as never }), - ), - ).rejects.toSatisfy(codeOf('invalid_targets')); - }); - - it('refuses an empty target set', async () => { - await expect( - durable().create(input({ targets: [] })), - ).rejects.toSatisfy(codeOf('invalid_targets')); - }); - - it('refuses a `single` row that wants a device notification', async () => { - await expect( - durable().create( - input({ - delivery: 'single', - handlerName: 'onWrite', - targets: ['socket', 'push'], - }), - ), - ).rejects.toSatisfy(codeOf('invalid_targets')); - await expect(durable().countForHolder(userId)).resolves.toMatchObject({ - total: 0, - }); - }); - - it('refuses an app`s `single` row with no worker to fall back to', async () => { - await expect( - durable().create( - input({ - appUid: 'app-single-rule', - delivery: 'single', - handlerName: 'onWrite', - targets: ['socket'], - }), - ), - ).rejects.toSatisfy(codeOf('invalid_targets')); - }); - - it('lets an account`s own `single` row stand on the socket alone', async () => { - const { row } = await durable().create( - input({ - delivery: 'single', - handlerName: 'onWrite', - targets: ['socket'], - }), - ); - expect(row.targets).toEqual(['socket']); - }); - - it('keeps the same targets on a `broadcast` row, where push is fine', async () => { - const { row } = await durable().create( - input({ targets: ['socket', 'push'] }), - ); - expect(row.targets).toEqual(['socket', 'push']); - }); - - it('refuses a `worker` target on a row with no app', async () => { - // Exactly one events worker per app: a row nobody's app made has - // nothing for that target to invoke. - await expect( - durable().create(input({ appUid: null, targets: ['socket', 'worker'] })), - ).rejects.toSatisfy(codeOf('invalid_targets')); - await expect(durable().countForHolder(userId)).resolves.toMatchObject({ - total: 0, - }); - }); - - it('allows a `worker` target once the row has an app', async () => { - const { row } = await durable().create( - input({ - appUid: `app-${uuidv4()}`, - targets: ['socket', 'worker'], - }), - ); - expect(row.targets).toEqual(['socket', 'worker']); - }); - it('refuses a context past the hard cap', async () => { await expect( durable().create(input({ context: 'x'.repeat(4097) })), @@ -704,18 +653,17 @@ describe('listHolderIdsForApp', () => { it('names every distinct holder bound to the app`s handlers', async () => { const appUid = `app-${uuidv4()}`; await durable().create( - input({ appUid, holderUserId: userId, targets: ['socket', 'worker'] }), + input({ appUid, holderUserId: userId }), ); await durable().create( input({ appUid, holderUserId: otherUserId, - targets: ['socket', 'worker'], }), ); // A second row for the same holder must not double the name. await durable().create( - input({ appUid, holderUserId: userId, targets: ['socket', 'worker'] }), + input({ appUid, holderUserId: userId }), ); const holders = await durable().listHolderIdsForApp(appUid); @@ -725,7 +673,7 @@ describe('listHolderIdsForApp', () => { it('finds a suspended row`s holder too', async () => { const appUid = `app-${uuidv4()}`; const { row } = await durable().create( - input({ appUid, targets: ['socket', 'worker'] }), + input({ appUid }), ); await durable().suspend([row], 'handler_not_found'); @@ -743,7 +691,7 @@ describe('reapForApp', () => { it('deletes every row bound to the app and bumps its owner`s generation', async () => { const appUid = `app-${uuidv4()}`; const mine = await durable().create( - input({ appUid, targets: ['socket', 'worker'] }), + input({ appUid }), ); const untouched = await durable().create(input()); const before = await cache().getGeneration(userId); @@ -762,9 +710,9 @@ describe('reapForApp', () => { it('never touches another app`s rows', async () => { const appUid = `app-${uuidv4()}`; const otherAppUid = `app-${uuidv4()}`; - await durable().create(input({ appUid, targets: ['socket', 'worker'] })); + await durable().create(input({ appUid })); const theirs = await durable().create( - input({ appUid: otherAppUid, targets: ['socket', 'worker'] }), + input({ appUid: otherAppUid }), ); await durable().reapForApp(appUid, 500); diff --git a/src/backend/stores/events/DurableSubscriptionStore.ts b/src/backend/stores/events/DurableSubscriptionStore.ts index 36f1f9adb..ae6e757a6 100644 --- a/src/backend/stores/events/DurableSubscriptionStore.ts +++ b/src/backend/stores/events/DurableSubscriptionStore.ts @@ -36,14 +36,7 @@ import { EVENT_SUBSCRIPTION_WIDTHS, } from './columnWidths.js'; import type { GenerationBump } from './EventSubscriptionStore.js'; -import { - isSubscriptionTarget, - SUBSCRIPTION_TARGETS, - targetsAllowedForDelivery, - type DurableSubscription, - type SubscriptionPermission, - type SubscriptionTarget, -} from './types.js'; +import type { DurableSubscription, SubscriptionPermission } from './types.js'; /** * Subscriptions that outlive the connection that made them. @@ -94,7 +87,6 @@ export interface DurableSubscriptionInput { match: string | null; op: FsOp | null; delivery: DeliveryClass; - targets: SubscriptionTarget[]; handlerName: string | null; context: string | null; permission: SubscriptionPermission; @@ -161,27 +153,6 @@ const contextTooLarge = (): HttpError => { legacyCode: 'events_context_too_large' }, ); -const invalidTargets = (): HttpError => - new HttpError( - 400, - `targets must be a subset of ${SUBSCRIPTION_TARGETS.join(', ')}`, - { legacyCode: 'invalid_targets' }, - ); - -const pushOnSingle = (): HttpError => - new HttpError( - 400, - 'A `single` subscription may not target `push`, and an app`s needs a `worker` target', - { legacyCode: 'invalid_targets' }, - ); - -const workerNeedsApp = (): HttpError => - new HttpError( - 400, - 'A subscription with no app has no events worker to target', - { legacyCode: 'invalid_targets' }, - ); - const quotaReached = (limit: number, scope: 'account' | 'app'): HttpError => new HttpError( 429, @@ -218,23 +189,11 @@ const asNumber = (value: unknown): number | null => { }; /** - * `targets` is a JSON column on mysql and postgres and text on sqlite, so the - * driver hands back either an array or the string it was stored as. + * The `targets` column predates delivery to every transport. Nothing reads it; + * it is written with what older servers expect for this row. */ -const parseTargets = (value: unknown): SubscriptionTarget[] => { - const raw = - typeof value === 'string' - ? (() => { - try { - return JSON.parse(value) as unknown; - } catch { - return []; - } - })() - : value; - if (!Array.isArray(raw)) return []; - return raw.filter(isSubscriptionTarget); -}; +const legacyTargets = (appUid: string | null): string => + JSON.stringify(appUid === null ? ['socket'] : ['socket', 'worker']); const toRow = (row: Record): DurableSubscription => ({ durable: true, @@ -255,8 +214,7 @@ const toRow = (row: Record): DurableSubscription => ({ ? null : String(row.app_uid), permission: String(row.permission), - delivery: String(row.delivery) as DeliveryClass, - targets: parseTargets(row.targets), + delivery: row.delivery === 'single' ? 'single' : 'broadcast', handlerName: row.handler_name === null || row.handler_name === undefined ? null @@ -278,7 +236,7 @@ const toRow = (row: Record): DurableSubscription => ({ const SELECT_COLUMNS = '`id`, `sub_id`, `token`, `owner_user_id`, `holder_user_id`, `app_uid`, ' + '`subject`, `anchor_uid`, `anchor_path`, `match`, `delivery`, `ops`, ' + - '`handler_name`, `targets`, `context`, `permission`, `include_value`, ' + + '`handler_name`, `context`, `permission`, `include_value`, ' + '`suspended_at`, `suspended_reason`, `expires_at`, `created_at`'; export class DurableSubscriptionStore extends PuterStore { @@ -292,11 +250,6 @@ export class DurableSubscriptionStore extends PuterStore { async create( input: DurableSubscriptionInput, ): Promise<{ row: DurableSubscription; bump: GenerationBump }> { - const targets = this.#assertTargets( - input.delivery, - input.targets, - input.appUid, - ); this.#assertContext(input.context); assertColumnWidths(EVENT_SUBSCRIPTION_WIDTHS, { token: input.token, @@ -326,7 +279,6 @@ export class DurableSubscriptionStore extends PuterStore { appUid: input.appUid, permission: input.permission, delivery: input.delivery, - targets, handlerName: input.handlerName, context: input.context, ...(input.includeValue ? { includeValue: true } : {}), @@ -350,7 +302,7 @@ export class DurableSubscriptionStore extends PuterStore { delivery: row.delivery, ops: row.op, handler_name: row.handlerName, - targets: JSON.stringify(row.targets), + targets: legacyTargets(row.appUid), context: row.context, permission: row.permission, include_value: row.includeValue ? 1 : 0, @@ -924,30 +876,6 @@ export class DurableSubscriptionStore extends PuterStore { return rows.length; } - /** - * The row cannot exist with transports its delivery class cannot use, or - * with a `worker` target and no app to run one for — "one events worker per - * app" means no app is no worker target, not a worker with nowhere to go. - * Held here rather than only at the API, so a writer that never passes - * through one cannot leave an unsatisfiable row behind. - */ - #assertTargets( - delivery: DeliveryClass, - targets: readonly string[], - appUid: string | null, - ): SubscriptionTarget[] { - if (!Array.isArray(targets) || targets.length === 0) - throw invalidTargets(); - if (!targets.every(isSubscriptionTarget)) throw invalidTargets(); - - const unique = [...new Set(targets as SubscriptionTarget[])]; - if (!targetsAllowedForDelivery(delivery, unique, appUid)) - throw pushOnSingle(); - if (appUid === null && unique.includes('worker')) - throw workerNeedsApp(); - return unique; - } - #assertContext(context: string | null): void { if (context === null) return; if (Buffer.byteLength(context, 'utf8') > DURABLE_CONTEXT_MAX_BYTES) diff --git a/src/backend/stores/events/EventSubscriptionStore.test.ts b/src/backend/stores/events/EventSubscriptionStore.test.ts index fad7f61da..044d64382 100644 --- a/src/backend/stores/events/EventSubscriptionStore.test.ts +++ b/src/backend/stores/events/EventSubscriptionStore.test.ts @@ -661,7 +661,6 @@ describe('durable rows sharing a session key', () => { socketId: undefined, durable: true, delivery: 'broadcast', - targets: ['socket'], handlerName: null, context: null, expiresAt: null, @@ -876,7 +875,6 @@ describe('reading rows for dispatch', () => { socketId: undefined, durable: true, delivery: 'broadcast', - targets: ['socket'], handlerName: null, context: null, expiresAt: null, diff --git a/src/backend/stores/events/PendingDeliveryStore.test.ts b/src/backend/stores/events/PendingDeliveryStore.test.ts index 881cac0ed..ca1b150c1 100644 --- a/src/backend/stores/events/PendingDeliveryStore.test.ts +++ b/src/backend/stores/events/PendingDeliveryStore.test.ts @@ -24,6 +24,7 @@ import { EVENTS_REGION_PENDING_CEILING, EVENTS_RETRY_BASE_MS, EVENTS_RETRY_MAX_MS, + EVENTS_SOCKET_LEASE_MS, deliveryBackoffMs, } from '../../controllers/events/limits.js'; import type { ProjectedEvent } from '../../services/events/registry.js'; @@ -195,33 +196,75 @@ describe('handing a delivery out', () => { }); }); - it('counts the socket attempts a delivery has spent', async () => { + it('hands a delivery to a socket under the socket lease, and counts it', async () => { const { entryId } = await store.enqueue(subId, event('a')); - await store.claim(subId, { leaseMs: 1 }); + await store.claim(subId); - await expect(store.recordSocketAttempt(subId, entryId)).resolves.toBe( - 1, - ); + await store.leaseToSocket(subId, entryId); vi.useFakeTimers({ toFake: ['Date'] }); - vi.setSystemTime(Date.now() + 1_000); + const start = Date.now(); + vi.setSystemTime(start + EVENTS_SOCKET_LEASE_MS - 1_000); + await expect(store.claim(subId)).resolves.toBeNull(); + vi.setSystemTime(start + EVENTS_SOCKET_LEASE_MS + 1_000); await expect(store.claim(subId)).resolves.toMatchObject({ entryId, socketAttempts: 1, }); }); + it('takes a declined delivery back once, under the worker lease', async () => { + const { entryId } = await store.enqueue(subId, event('a')); + await store.claim(subId); + await store.leaseToSocket(subId, entryId); + + await expect( + store.takeFromSocket(subId, entryId), + ).resolves.toMatchObject({ + entryId, + socketAttempts: 1, + }); + // A second decline for the same lease changes nothing. + await expect(store.takeFromSocket(subId, entryId)).resolves.toBeNull(); + + // Held past the socket lease: the worker has it now. + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(Date.now() + EVENTS_SOCKET_LEASE_MS + 1_000); + await expect(store.claim(subId)).resolves.toBeNull(); + }); + + it('ignores a decline for a lease that lapsed and was claimed again, or settled', async () => { + const { entryId } = await store.enqueue(subId, event('a')); + await store.claim(subId); + await store.leaseToSocket(subId, entryId); + + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(Date.now() + EVENTS_SOCKET_LEASE_MS + 1_000); + await expect(store.claim(subId)).resolves.toMatchObject({ entryId }); + await expect(store.takeFromSocket(subId, entryId)).resolves.toBeNull(); + + await store.settle(subId, entryId); + await expect(store.takeFromSocket(subId, entryId)).resolves.toBeNull(); + }); + + it('ignores a decline for a delivery never handed to a socket', async () => { + const { entryId } = await store.enqueue(subId, event('a')); + await store.claim(subId); + + await expect(store.takeFromSocket(subId, entryId)).resolves.toBeNull(); + }); + it('records remote regions tried separately from socket attempts', async () => { const { entryId } = await store.enqueue(subId, event('a')); await store.claim(subId, { leaseMs: 1 }); - await store.recordSocketAttempt(subId, entryId); + await store.leaseToSocket(subId, entryId); await expect( store.recordRemoteAttempt(subId, entryId, 'east'), ).resolves.toEqual(['east']); vi.useFakeTimers({ toFake: ['Date'] }); - vi.setSystemTime(Date.now() + 1_000); + vi.setSystemTime(Date.now() + EVENTS_SOCKET_LEASE_MS + 1_000); await expect(store.claim(subId)).resolves.toMatchObject({ entryId, socketAttempts: 1, @@ -232,13 +275,13 @@ describe('handing a delivery out', () => { it('gives a delivery a fresh attempt budget on request', async () => { const { entryId } = await store.enqueue(subId, event('a')); await store.claim(subId, { leaseMs: 1 }); - await store.recordSocketAttempt(subId, entryId); + await store.leaseToSocket(subId, entryId); await store.recordRemoteAttempt(subId, entryId, 'east'); await store.resetSocketAttempts(subId, entryId); vi.useFakeTimers({ toFake: ['Date'] }); - vi.setSystemTime(Date.now() + 1_000); + vi.setSystemTime(Date.now() + EVENTS_SOCKET_LEASE_MS + 1_000); await expect(store.claim(subId)).resolves.toMatchObject({ entryId, socketAttempts: 0, @@ -493,7 +536,8 @@ describe('finding the work', () => { it('never scans the keyspace, whatever it is asked to do', async () => { const { entryId } = await store.enqueue(subId, event('a')); await store.claim(subId); - await store.recordSocketAttempt(subId, entryId); + await store.leaseToSocket(subId, entryId); + await store.takeFromSocket(subId, entryId); await store.settle(subId, entryId); await store.enqueue(subId, event('b')); await store.purge(subId); diff --git a/src/backend/stores/events/PendingDeliveryStore.ts b/src/backend/stores/events/PendingDeliveryStore.ts index 88bb6dccb..aacffa718 100644 --- a/src/backend/stores/events/PendingDeliveryStore.ts +++ b/src/backend/stores/events/PendingDeliveryStore.ts @@ -23,6 +23,7 @@ import { EVENTS_FAILURE_COUNTER_TTL_MS, EVENTS_PENDING_DELIVERIES_PER_SUBSCRIPTION, EVENTS_REGION_PENDING_CEILING, + EVENTS_SOCKET_LEASE_MS, deliveryBackoffMs, } from '../../controllers/events/limits.js'; import type { @@ -157,6 +158,19 @@ end return head[2] `; +/** + * Move one lease, only if it is still there and, when `expected` is given, + * still the lease that was handed out. KEYS: lease. ARGV: entryId, expected + * (empty for any), leaseUntil. Returns 1 when moved. + */ +const RELEASE_SCRIPT = ` +local current = redis.call('ZSCORE', KEYS[1], ARGV[1]) +if not current then return 0 end +if ARGV[2] ~= '' and tonumber(current) ~= tonumber(ARGV[2]) then return 0 end +redis.call('ZADD', KEYS[1], ARGV[3], ARGV[1]) +return 1 +`; + interface PendingScripts { pendingClaim( entries: string, @@ -172,6 +186,12 @@ interface PendingScripts { lease: string, hold: string, ): Promise; + pendingRelease( + lease: string, + entryId: string, + expected: string, + leaseUntil: string, + ): Promise; } // -- Shapes ----------------------------------------------------------- @@ -220,6 +240,8 @@ interface StoredEntry { billed?: boolean; /** Absent for an event no handler produced. */ handlerDepth?: number; + /** When the socket lease it was last handed out under expires. */ + socketLeaseUntil?: number; } const parseEntry = (raw: string | null): StoredEntry | null => { @@ -274,15 +296,18 @@ export class PendingDeliveryStore extends PuterStore { numberOfKeys: 4, lua: REINDEX_SCRIPT, }); + this.clients.redis.defineCommand('pendingRelease', { + numberOfKeys: 1, + lua: RELEASE_SCRIPT, + }); } return this.clients.redis as unknown as PendingScripts; } /** - * Long enough for a handler doing real work — twice the invoke timeout the - * invoker resolves the same way, so a slow but successful run is never - * re-invoked mid-flight — and short enough that a tab that closed - * mid-delivery does not hold the queue. + * The worker lease: twice the invoke timeout the invoker resolves the same + * way, so a slow but successful run is never re-invoked mid-flight. A + * client gets the shorter socket lease instead (`leaseToSocket`). */ #leaseTtlMs(): number { const configured = this.config.events?.invokeTimeoutMs; @@ -379,20 +404,64 @@ export class PendingDeliveryStore extends PuterStore { await this.clients.redis.zadd(INDEX_KEY, 'XX', Date.now(), subId); } - /** Count one socket attempt against a claimed delivery. */ - async recordSocketAttempt(subId: string, entryId: string): Promise { + /** + * Hand a claimed delivery to a connected client: count the attempt and + * shorten its lease to the socket lease. The expiry is stamped on the + * entry, which is how a decline later proves the lease is still the + * client's. + */ + async leaseToSocket(subId: string, entryId: string): Promise { const entry = parseEntry( await this.clients.redis.hget(entriesKey(subId), entryId), ); - if (!entry) return 0; + if (!entry) return; - const socketAttempts = entry.socketAttempts + 1; + const socketLeaseUntil = Date.now() + EVENTS_SOCKET_LEASE_MS; await this.clients.redis.hset( entriesKey(subId), entryId, - JSON.stringify({ ...entry, socketAttempts }), + JSON.stringify({ + ...entry, + socketAttempts: entry.socketAttempts + 1, + socketLeaseUntil, + }), ); - return socketAttempts; + await this.#scripts().pendingRelease( + leaseKey(subId), + entryId, + '', + String(socketLeaseUntil), + ); + } + + /** + * Take back a delivery a client declined, under a worker lease. Null when + * the client's lease is no longer the one in place: settled, lapsed and + * claimed again, or already declined. + */ + async takeFromSocket( + subId: string, + entryId: string, + ): Promise { + const entry = parseEntry( + await this.clients.redis.hget(entriesKey(subId), entryId), + ); + if (!entry?.socketLeaseUntil) return null; + + const moved = await this.#scripts().pendingRelease( + leaseKey(subId), + entryId, + String(entry.socketLeaseUntil), + String(Date.now() + this.#leaseTtlMs()), + ); + if (Number(moved) !== 1) return null; + return { + entryId, + event: entry.event, + socketAttempts: entry.socketAttempts, + triedRegions: entry.triedRegions ?? [], + handlerDepth: entry.handlerDepth ?? 0, + }; } /** diff --git a/src/backend/stores/events/types.ts b/src/backend/stores/events/types.ts index f15c60309..0337862e8 100644 --- a/src/backend/stores/events/types.ts +++ b/src/backend/stores/events/types.ts @@ -29,43 +29,14 @@ import type { FsOp } from '../../services/events/subjects.js'; * is declared once here rather than per store. */ -/** Transports a delivery may take. */ -export const SUBSCRIPTION_TARGETS = ['socket', 'worker', 'push'] as const; - -export type SubscriptionTarget = (typeof SUBSCRIPTION_TARGETS)[number]; - -export const isSubscriptionTarget = ( - value: unknown, -): value is SubscriptionTarget => - SUBSCRIPTION_TARGETS.includes(value as SubscriptionTarget); - -/** Transports a session row may take: it has one connection and no handler. */ -export const SESSION_TARGETS: SubscriptionTarget[] = ['socket']; - -/** Transports a durable row takes unless the caller says otherwise. */ -export const DEFAULT_DURABLE_TARGETS: SubscriptionTarget[] = [ - 'socket', - 'worker', -]; - /** - * Whether a delivery class may carry these transports. `single` and `push` are - * incompatible by construction — a lease can only be settled by a consumer that - * reports back, and a device notification never does. An app's `single` row - * also needs a `worker` to fall back to once the connected clients have had - * their turns, or an unacknowledged delivery sits at the head of its queue for - * good. An account's own row has no app to run a worker for, so it is held on - * the socket alone and the pending hold is its backstop. Both are invariants - * every writer is held to, not defaults. + * Whether a row's events also run the app's published handler in its events + * worker. Only an app has a worker, and only a named handler can be run. */ -export const targetsAllowedForDelivery = ( - delivery: DeliveryClass, - targets: readonly SubscriptionTarget[], - appUid: string | null, -): boolean => - delivery !== 'single' || - (!targets.includes('push') && - (appUid === null || targets.includes('worker'))); +export const runsInWorker = (row: { + appUid: string | null; + handlerName?: string | null; +}): boolean => row.appUid !== null && !!row.handlerName; /** * What a row's delivery re-check runs against. An access mode for a filesystem @@ -104,8 +75,6 @@ export interface DispatchSubscription { appUid: string | null; /** What the subscribe check passed under; re-checked per delivery. */ permission: SubscriptionPermission; - /** Transports this row's deliveries may take. */ - targets?: SubscriptionTarget[]; /** KV rows only: deliveries carry the key's new value where they can. */ includeValue?: true; /** Session rows only: the connection a delivery is addressed at. */ @@ -126,7 +95,6 @@ export interface SessionSubscription extends DispatchSubscription { export interface DurableSubscription extends DispatchSubscription { durable: true; delivery: DeliveryClass; - targets: SubscriptionTarget[]; handlerName: string | null; /** Plaintext JSON, capped at 4 KB. Never returned by `list`. */ context: string | null; diff --git a/src/docs/src/Events.md b/src/docs/src/Events.md index 5e1e1b97d..14b6a4b1f 100644 --- a/src/docs/src/Events.md +++ b/src/docs/src/Events.md @@ -11,7 +11,7 @@ The Events API tells your app when a user's data changes. Subscribe to a file, a There are two kinds of subscription: - **Session subscriptions** ([`onLocal()`](/Events/onLocal/)) live on the open page and end when it closes. Use them to keep a UI in sync. -- **Persistent subscriptions** ([`onPersistent()`](/Events/onPersistent/)) are stored on the account and keep running while your app is closed, running a [handler](/Events/handlers/) your app publishes. Use them for background work, such as processing uploads as they arrive. +- **Persistent subscriptions** ([`onPersistent()`](/Events/onPersistent/)) are stored on the account and keep running while your app is closed, running a [handler](/Events/handlers/) your app publishes. Use them for background work, such as processing uploads as they arrive. For work that has to finish, pass `queue: true` and an event whose run never finishes, or fails with a temporary error, is offered again. See [Choosing a mode](/Events/onPersistent/#choosing-a-mode). What you watch is written as a subject string: @@ -212,7 +212,7 @@ With the [User-Pays Model](/user-pays-model/), deliveries are billed to the user }); for (const row of await puter.events.list()) { - puter.print(`${row.subject} — ${row.delivery}`); + puter.print(`${row.subject} — ${row.queue ? 'queue' : 'every listener'}`); puter.print(` (context: ${row.contextKeys?.join(', ') ?? 'none'})
`); } diff --git a/src/docs/src/Events/handlers.md b/src/docs/src/Events/handlers.md index 023c247ff..5328a530a 100644 --- a/src/docs/src/Events/handlers.md +++ b/src/docs/src/Events/handlers.md @@ -102,22 +102,20 @@ Renaming is publishing the new name and removing the old one. Subscriptions don' An app's first published handler creates its [events worker](/Events/workers/), and removing the last one takes it down. -### Refusing a delivery +### Retrying a delivery -In the events worker, a handler that returns takes the delivery, and one that throws is retried later. When retrying can't help (a malformed event, say), throw an error with `terminal: true` or `code: 'events_terminal'`. The delivery is dropped and replaced by a [gap marker](/Events/subjects/#gaps) with `reason: 'handler_rejected'`. +A handler that returns takes the delivery. On a `queue` subscription, a handler that throws drops the event, and it's replaced by a [gap marker](/Events/subjects/#gaps) with `reason: 'handler_rejected'`. Network errors, timeouts, rate limits and `5xx` errors are retried. To have any other error retried, throw it with `retry: true`: ```js -await puter.events.handlers.publish('ingestUpload', async ({ event }) => { - if (! event.path.endsWith('.json')) { - const err = new Error(`cannot ingest ${event.path}`); - err.terminal = true; - throw err; +await puter.events.handlers.publish('ingestUpload', async ({ event, ctx }) => { + const res = await fetch(ctx.endpoint, { method: 'POST', body: event.path }); + if (res.status === 409) { + throw Object.assign(new Error('ingest busy'), { retry: true }); } - // ... }, { appUid }); ``` -See [`onPersistent()`](/Events/onPersistent/#where-the-handler-runs) for the full list of outcomes, and [suspended subscriptions](/Events/onPersistent/#suspended-subscriptions) for what happens to a suspended subscription's backlog. +See [what happens when a handler fails](/Events/onPersistent/#what-happens-when-a-handler-fails) for the full list of outcomes, and [suspended subscriptions](/Events/onPersistent/#suspended-subscriptions) for what happens to a suspended subscription's backlog. ## Errors diff --git a/src/docs/src/Events/list.md b/src/docs/src/Events/list.md index b3049b442..ea9ac2366 100644 --- a/src/docs/src/Events/list.md +++ b/src/docs/src/Events/list.md @@ -39,7 +39,6 @@ Each subscription is the object [`onPersistent()`](/Events/onPersistent/) return - `contextKeys` (Array | null) and `contextHash` (String | null) describe the stored `context`. **Values are never returned**, since context often holds secrets. The hash changes whenever a value does. - `suspendedAt` (Number | null) and `suspendedReason` (String | null) are set when a subscription is [suspended](/Events/onPersistent/#suspended-subscriptions): `handler_not_found`, `failures`, `no_credit`, or `permission_revoked`. -- `targets` (Array) can include `'push'`, which is accepted but delivers nothing yet. The promise rejects with `{ message, code }`: `auth_canceled` if nobody was signed in and the user closed the sign-in, `too_many_requests` over the listing rate limit, `events_disabled` where events are off, and `events_failed` for a response the SDK couldn't read. @@ -61,7 +60,7 @@ The promise rejects with `{ message, code }`: `auth_canceled` if nobody was sign }); for (const row of await puter.events.list()) { - puter.print(`${row.subject} — ${row.delivery}`); + puter.print(`${row.subject} — ${row.queue ? 'queue' : 'every listener'}`); puter.print(` (context: ${row.contextKeys?.join(', ') ?? 'none'})
`); } diff --git a/src/docs/src/Events/onPersistent.md b/src/docs/src/Events/onPersistent.md index 2cc31022d..c1da29e6c 100644 --- a/src/docs/src/Events/onPersistent.md +++ b/src/docs/src/Events/onPersistent.md @@ -6,7 +6,7 @@ platforms: [websites, apps, nodejs, workers]
The Events API is in beta. Event shapes, limits, and behavior may change between releases.
-Creates a subscription that outlives this connection. It's stored on the account, keeps matching while your app is closed, and runs a handler your app published with [`puter.events.handlers.publish()`](/Events/handlers/). Compare [`puter.events.onLocal()`](/Events/onLocal/), which ends with the page. +Creates a subscription that outlives this connection. It's stored on the account, keeps matching while your app is closed, and runs a handler your app published with [`puter.events.handlers.publish()`](/Events/handlers/). Each event also reaches every connected client. Compare [`puter.events.onLocal()`](/Events/onLocal/), which ends with the page. The subscription is live immediately in the region where it was created. Changes made in other regions can take a moment longer (usually well under a second) to reach it right after this call resolves. @@ -24,36 +24,52 @@ puter.events.onPersistent(options) #### `options` (Object) (required) - `subject` (String) (required): What to watch, in the same form `onLocal()` takes, e.g. `fs:~/Documents` or `fs:~/inbox/*.json:add`. -- `delivery` (String): `'broadcast'` (default) delivers to every listener. `'single'` delivers each event to exactly one consumer, which must acknowledge it, and requires `handlerName`. -- `targets` (Array): Where deliveries may go: any of `'socket'`, `'worker'`, `'push'`. Defaults to `['socket', 'worker']` when an app subscribes and `['socket']` when an account session with no app does. A subscription with no app can't target `'worker'`, because the [events worker](/Events/workers/) belongs to an app. `'push'` is reserved for future device notifications: it's accepted (except with `single`) but delivers nothing yet. -- `handlerName` (String): The published handler to run. Required for `single`. -- `handler` (Function | String | Object): The handler source you wrote this subscription against: a function, a source string, or `{ file: '~/AppData/…/handler.js' }`. Only its **hash** is sent. The subscription is created only if it matches what's published under `handlerName`, so `handlerName` is required with it. Passing a function also runs it in this client, on the deliveries the server leaves to clients (see below). +- `queue` (Boolean): Handles events like a work queue. Each event goes to one consumer at a time, and it's offered again if a run never finishes or hits a temporary error. Requires `handlerName`. Defaults to `false`. See [Choosing a mode](#choosing-a-mode). +- `delivery` (String): Deprecated, use `queue` instead. A value of `'single'` is the same as `queue: true`, and `'broadcast'` is the default. +- `targets` (Array): Deprecated and ignored. +- `handlerName` (String): The published handler to run in your app's [events worker](/Events/workers/). Required for `queue`. A subscription with no app has no events worker, so its handler runs only in connected clients. +- `handler` (Function | String | Object): The handler source you wrote this subscription against: a function, a source string, or `{ file: '~/AppData/…/handler.js' }`. Only its **hash** is sent. The subscription is created only if it matches what's published under `handlerName`, so `handlerName` is required with it. Passing a function also runs it in this client for each delivery it receives (see below). - `context` (Object): Values the handler needs, passed to it as a frozen `ctx`. **Up to 4 KB serialized**; see below. - `expiresAt` (Number | String): When the subscription ends on its own, as unix seconds or an ISO-8601 string. Must be in the future. - `includeValue` (Boolean): For a `kv:` subject, include the key's new value as `event.value` (`null` on a `del`, absent on an `expire`). The value is left out when it's over 16 KB, when more than 128 subscriptions match the change in that region, or when the filter-check limit stops the count early. Refused on other subjects. -- `onError` (Function): Called with `{ message, code }` when this client stops running `handler` because its connection can't be restored (`reauth_required` after a sign-out, `events_connection_failed` otherwise). The subscription itself keeps going (in the events worker, if it targets `worker`), and resumes taking deliveries here once this client reconnects. Only used with a function `handler`; without it, the stop is logged to the console. +- `onError` (Function): Called with `{ message, code }` when this client stops running `handler` because its connection can't be restored (`reauth_required` after a sign-out, `events_connection_failed` otherwise). The subscription itself keeps going in the events worker, and resumes taking deliveries here once this client reconnects. Only used with a function `handler`; without it, the stop is logged to the console. ## Background delivery needs consent -Running your handler while the user isn't there needs its own per-app permission, **`events:background`**. Subscribing with `worker` among the `targets` (the default for an app) without it fails with `events_background_consent_required`. To ask for it, call [`puter.perms.request()`](/Perms/request/) with the `'backgroundEvents'` resource: +Running your handler while the user isn't there needs its own per-app permission, **`events:background`**. When an app subscribes with a `handlerName` without it, the call fails with `events_background_consent_required`. To ask for it, call [`puter.perms.request()`](/Perms/request/) with the `'backgroundEvents'` resource: ```js await puter.perms.request('backgroundEvents'); ``` -The user can revoke it wherever they manage the app's access. That suspends every worker-target subscription the app holds for them with `permission_revoked`, and granting it again doesn't resume them: subscribe again. A subscription that only wants deliveries while your app is open needs no consent: pass `targets: ['socket']`. +The user can revoke it wherever they manage the app's access. That suspends every subscription with a `handlerName` the app holds for them with `permission_revoked`, and granting it again doesn't resume them: subscribe again. A subscription without a `handlerName` runs nothing in the background and needs no consent. Background handlers run as a worker session for your app, which appears in the user's sessions list. Revoking that session, withdrawing `events:background`, uninstalling the app, destroying its events worker, or deleting the app all stop background deliveries and invalidate the session's token. +## Choosing a mode + +| | Who gets each event | If a run fails | +|---|---|---| +| [`onLocal()`](/Events/onLocal/) | Clients connected right now | Gone. Nothing is kept while disconnected. | +| `onPersistent()` | The events worker, even with the app closed, plus every connected client | Gone. Each place gets one attempt. | +| `onPersistent({ queue: true })` | One consumer at a time: a connected client first, then the events worker | Offered again only if the run never finished or hit a network or other temporary error. | + +- **[`onLocal()`](/Events/onLocal/)** is live and best effort. Use it to update the page while it's open. +- **`onPersistent()`** runs your handler in the events worker for every event, even when your app is closed, and also sends the event to every connected client. Each place gets one attempt, and a failed run isn't retried. Use it to react to every change, such as keeping an index or a notification up to date. +- **`onPersistent({ queue: true })`** hands each event to one consumer at a time and retries it when the run never finished or failed for a temporary reason. Use it for work that has to finish, such as processing every uploaded file or calling a webhook. + +A queued event comes back only on a retry or a lost answer, with the same `event.id`, so check that ID before doing work twice. Open tabs that passed a function `handler` all receive it, like a broadcast. + ## Where the handler runs -A connected client always *receives* the event, but doesn't always run the handler for it. A `broadcast` delivery runs in the events worker or in the connected clients, never both: +Without `queue`, every event runs the handler in each place that's listening: -| Delivery | Runs | -| --- | --- | -| `broadcast`, default `targets` (`['socket', 'worker']`) | The app's [events worker](/Events/workers/). Connected clients run it instead only when the worker can't take it: no worker runtime on this deployment, or the [handler-run limit](/rate-limits-and-quotas/#events) for this user and app is used up for the minute. A run that fails in the worker isn't retried in a client. | -| `broadcast`, `targets: ['socket']` | Every connected client. Nothing runs it otherwise. | -| `single` | Connected clients first; the worker once sockets are spent. It's offered again until acknowledged (see [below](#acknowledging-a-single-delivery)). | +- The app's [events worker](/Events/workers/) runs the handler named by `handlerName`, whether or not a client is open. +- Every connected client that passed a function `handler` runs it too. Every [`onLocal()`](/Events/onLocal/) subscription that matches the change gets the event as well. + +The events worker skips a run when the deployment has no worker runtime or the [handler-run limit](/rate-limits-and-quotas/#events) for this user and app is used up for the minute. Connected clients still run it. + +With `queue: true`, a connected client that passed a function `handler` gets the first try. If no client has one, or its run fails, the events worker takes the event right away. A client that doesn't answer within 15 seconds loses the event to the events worker, so keep client handlers short. If the user has your app open in several tabs, each tab that passed a function `handler` gets the event, as a broadcast; keep side effects that must happen once in the worker handler. Whichever place runs it, it's the same code, called with: @@ -63,7 +79,7 @@ Whichever place runs it, it's the same code, called with: | `ctx` | The frozen `context` this subscription was created with. | | `user` | A `puter` bound to the account holding the subscription, with the same access your app has for that user in a tab. In a client, it's the ambient `puter`. | | `fetch` | [`puter.net.fetch`](/Networking/fetch/) where it exists, otherwise the environment's `fetch`. | -| `ack` | On a `single` subscription only; see below. | +| `ack` | On a `queue` subscription only. Optional; see [below](#what-happens-when-a-handler-fails). | Only a **function** `handler` runs in this client. A source string or `{ file }` is only used for its hash. The connection reconnects on its own when it drops, so this client keeps taking deliveries unless `onError` is called. @@ -71,27 +87,32 @@ Those five bindings are the handler's whole environment. The events worker has n ### Handlers that trigger handlers -In the events worker, a write made through `user`, or through a token `user` creates, is one run deeper than the event the handler ran for, and it can run handlers of its own, including this one. `user.workers.create` is refused instead — a handler cannot spin up a worker of its own to escape the chain. A chain stops at **12 runs on a paid plan and 4 on a free one**, by the plan of the account holding the subscription. An event past that runs no handler in the events worker. A `broadcast` one still reaches connected clients without running the handler there; a `single` one is still offered to a connected client first and runs there. The dropped run leaves no gap marker and doesn't count as a failure. Writes made anywhere else, including from a handler running in a client, start a new chain. See [handler chains](/rate-limits-and-quotas/#handler-chains). +In the events worker, a write made through `user`, or through a token `user` creates, is one run deeper than the event the handler ran for, and it can run handlers of its own, including this one. `user.workers.create` is refused instead — a handler cannot spin up a worker of its own to escape the chain. A chain stops at **12 runs on a paid plan and 4 on a free one**, by the plan of the account holding the subscription. An event past that runs no handler in the events worker. Without `queue`, it still reaches connected clients without running the handler there. A queued one is still offered to a connected client first and runs there. The dropped run leaves no gap marker and doesn't count as a failure. Writes made anywhere else, including from a handler running in a client, start a new chain. See [handler chains](/rate-limits-and-quotas/#handler-chains). `user` in the events worker is valid for 15 minutes, so don't keep it past the run. -### Acknowledging a `single` delivery +### What happens when a handler fails -A `single` delivery stays owed until it's acknowledged: +These rules apply to a `queue` subscription, in a client or in the events worker: -- Calling `ack()` takes the delivery. -- Returning **without** calling `ack()` also takes it. -- **Throwing takes nothing.** After 60 seconds the delivery is offered again, so a handler that throws sees the same event again. `event.id` stays the same across redeliveries; use it to skip work you've already done. - -In the events worker, the worker's response decides: - -| Response | Result | +| The handler | Result | | --- | --- | -| `2xx`: the handler returned | Delivery taken. | -| `4xx`: the handler threw an error with `terminal: true` or `code: 'events_terminal'` | Delivery dropped and replaced by a gap marker with `reason: 'handler_rejected'`. | -| `5xx` (any other thrown error), `429`, or no answer within 30 seconds | Retried after 2 seconds, doubling each time up to 5 minutes. | +| Resolves | Done. | +| Throws | The event is dropped and replaced by a [gap marker](/Events/subjects/#gaps) with `reason: 'handler_rejected'`. It counts as a failure. | +| Throws a temporary error: a network error, a timeout, a rate limit (`429`), a `5xx`, or any error with `retry: true` | Retried after 2 seconds, then 4, 8 and so on, up to 5 minutes apart. It counts as a failure. | +| Never answers (the tab closed, the worker crashed, or it ran past 30 seconds in the worker) | Offered again. | -**Five failures in a row, refusals included, suspend the subscription** with `failures`. The developer is notified, and publishing the handler again resumes it. A terminal error only matters in the events worker; thrown in a client, it just rejects like any other error. +To have an error retried, throw it with `retry: true`: + +```js +throw Object.assign(new Error('Service busy, try later'), { retry: true }); +``` + +**Five failures in a row suspend the subscription** with `failures`. The developer is notified, and publishing the handler again resumes it. + +Without `queue`, a failed run is never retried. + +Advanced: a handler can call `ack()` to mark the event done before it finishes. A throw after that doesn't undo it. ## `context` is captured once, up to 4 KB @@ -122,9 +143,10 @@ A suspended subscription stops delivering and isn't billed. Its backlog is cut t ## Delivery guarantees -Clients connect to the nearest region. Events reach a client wherever it's connected, and `ack()` works on any connection. +Clients connect to the nearest region. Events reach a client wherever it's connected. -- `single` deliveries are **at-least-once**: one may arrive twice, with the same `event.id`. Make handlers safe to run twice. +- Without `queue`, an event runs the handler in the events worker and in each connected client that passed a function `handler`. A run that fails or happens while nothing is connected isn't retried. +- `queue` deliveries are **at-least-once**: an event whose run never finished, or failed with a temporary error, arrives again with the same `event.id`. - Undelivered events are held in the region where the change happened. If that region goes down, only what it was holding is lost. - Order is kept within a region and is best-effort across regions. Coalescing (250 ms) also runs per region, so two quick writes can arrive as one event nearby and as two farther away. @@ -135,7 +157,7 @@ A `Promise` that resolves to the subscription: - `subId` (String): Its id, which [`puter.events.unsubscribe()`](/Events/unsubscribe/) takes. It never changes. - `subject` (String): For `kv:` and `notif:`, the subject in full form: `kv:cart` comes back as `kv::cart`. For `fs:`, as you passed it, except a relative path comes back resolved the same way [`onLocal()`](/Events/onLocal/) returns it. - `anchor`, `match`, `op`: as `onLocal()` returns them. -- `delivery` (String), `targets` (Array), `handlerName` (String | null), `includeValue` (Boolean). +- `queue` (Boolean), `handlerName` (String | null), `includeValue` (Boolean). - `appUid` (String | null): The app that created it, or `null` if an account session did. - `contextKeys` (Array | null), `contextHash` (String | null): the key names and a hash of the stored context, never its values. - `createdAt`, `expiresAt` (Number | null): unix seconds. @@ -154,12 +176,11 @@ The promise rejects with `{ message, code }`: | `events_handler_hash_unavailable` | This environment has no `crypto.subtle`, so `handler` can't be hashed. Pass `handlerName` alone. | | `events_handler_not_found` | Nothing is published under `handlerName`. The subscription isn't created. | | `events_handler_hash_mismatch` | The published handler isn't the source you passed as `handler`. | -| `events_handler_required` | `delivery: 'single'` without a `handlerName`. | -| `events_background_consent_required` | The subscription targets `worker` and the user hasn't granted `events:background`. | +| `events_handler_required` | `queue: true` without a `handlerName`. | +| `events_background_consent_required` | An app subscribed with a `handlerName` and the user hasn't granted `events:background`. | | `events_context_too_large` | The serialized `context` is over 4 KB. | | `events_context_invalid` | `context` is not JSON-serializable. | -| `invalid_targets` | `targets` is empty or not an array, it has an unknown target, it has `push` with `single`, an app's `single` subscription leaves out `worker`, or it has `worker` with no app. | -| `bad_request` | `delivery` isn't `'broadcast'` or `'single'`, or `handlerName` isn't a string or is over 128 characters. | +| `bad_request` | `queue` isn't a boolean, `delivery` isn't `'broadcast'` or `'single'`, the two disagree, or `handlerName` isn't a string or is over 128 characters. | | `invalid_expires_at` | `expiresAt` is not a future time. | | `invalid_include_value` | `includeValue` isn't a boolean, or the subject isn't `kv:`. | | `subject_does_not_exist` | The subject doesn't exist, or this account can't read it. | diff --git a/src/docs/src/Events/subjects.md b/src/docs/src/Events/subjects.md index 0376fb9ae..47c11ab95 100644 --- a/src/docs/src/Events/subjects.md +++ b/src/docs/src/Events/subjects.md @@ -198,7 +198,7 @@ Limits never make a subscription fail. Instead they send a **gap marker** in pla | `filter_evaluation_limit` | The event had more subscription filters to check than one event may check. | | `delivery_rate_limit` | This subscription went over its per-minute delivery rate. | | `backlog_overflow` | A persistent subscription's undelivered backlog was full, so the oldest deliveries were dropped. | -| `handler_rejected` | A persistent subscription's handler refused the delivery (a `4xx`, or a thrown terminal error). | +| `handler_rejected` | A `queue` subscription's handler threw an error that isn't retried. | | `suspended_backlog_expired` | A persistent subscription stayed [suspended](/Events/onPersistent/#suspended-subscriptions) past the time its backlog is kept. | | `reconnect` | The connection dropped and came back. Changes made while it was down weren't delivered. Sent once, after the subscription is restored. | diff --git a/src/docs/src/Events/workers.md b/src/docs/src/Events/workers.md index 0dc157aa2..ca4140d99 100644 --- a/src/docs/src/Events/workers.md +++ b/src/docs/src/Events/workers.md @@ -6,7 +6,7 @@ platforms: [websites, apps, nodejs, workers]
The Events API is in beta. Event shapes, limits, and behavior may change between releases.
-An **events worker** runs an app's published [handlers](/Events/handlers/): every `broadcast` delivery that targets it, whether or not a client is connected, and a `single` delivery once no connected client takes it. Each app has at most one, created when it publishes its first handler, however many handlers it publishes. +An **events worker** runs an app's published [handlers](/Events/handlers/): every event for a subscription with a `handlerName`, whether or not a client is connected, and a [`queue`](/Events/onPersistent/#choosing-a-mode) delivery once no connected client takes it. Each app has at most one, created when it publishes its first handler, however many handlers it publishes. A hosted Puter deployment may bill each app's events worker as a monthly cost, even if nothing ever delivers to it. Use this API to see which apps have one and remove the ones you don't need. diff --git a/src/docs/src/playground/examples/events-list.html b/src/docs/src/playground/examples/events-list.html index ac94152da..d7c3fd8fe 100644 --- a/src/docs/src/playground/examples/events-list.html +++ b/src/docs/src/playground/examples/events-list.html @@ -11,7 +11,7 @@ }); for (const row of await puter.events.list()) { - puter.print(`${row.subject} — ${row.delivery}`); + puter.print(`${row.subject} — ${row.queue ? 'queue' : 'every listener'}`); puter.print(` (context: ${row.contextKeys?.join(', ') ?? 'none'})
`); } diff --git a/src/docs/src/rate-limits-and-quotas.md b/src/docs/src/rate-limits-and-quotas.md index c2bfcbb93..be94f0d94 100644 --- a/src/docs/src/rate-limits-and-quotas.md +++ b/src/docs/src/rate-limits-and-quotas.md @@ -509,17 +509,17 @@ Unsubscribing counts toward its own `unsubscribe` limit, not the `subscribe` one | Limit | Value | Scope | | ---------------------------------------- | ---------- | ---------------------- | -| Broadcast deliveries | 600/min | Per subscription | -| `single` deliveries | 120/min | Per subscription | +| Deliveries without `queue` | 600/min | Per subscription | +| `queue` deliveries | 120/min | Per subscription | | Handler runs | 60/min | Per user, per app | | Coalescing window | 250 ms | Per subscription and subject | -| Time to acknowledge a `single` delivery | 60 seconds | Per delivery | +| Time for a client to answer a `queue` delivery | 15 seconds | Per delivery | | Undelivered backlog | 10,000 | Per subscription | | Undelivered backlog | 1,000,000 | Per region | | KV value inlined in a delivery | 16 KB | Per delivery | - Over a delivery rate, the event is replaced by a gap marker (`delivery_rate_limit`). -- Over the handler-run rate, a `single` delivery waits and goes out later, and a `broadcast` one runs only in connected clients. Neither counts as a handler failure. +- Over the handler-run rate, a `queue` delivery waits and goes out later, and any other delivery runs only in connected clients. Neither counts as a handler failure. - When a backlog is full, the oldest deliveries are dropped and replaced by one gap marker (`backlog_overflow`). #### Handlers @@ -532,7 +532,7 @@ Unsubscribing counts toward its own `unsubscribe` limit, not the `subscribe` one | Handlers per `publishAll()` call | 50 | Per call | | Events worker deploys | 30/hour | Per app, all users | | Run timeout | 30 seconds | Per run | -| Retry delay | 2 seconds, doubling, up to 5 minutes | Per delivery | +| Retry delay (`queue`, temporary errors) | 2 seconds, doubling, up to 5 minutes | Per delivery | | Failures in a row before suspension | 5 | Per subscription | | `user` token lifetime in the events worker | 15 minutes | Per run | @@ -547,7 +547,7 @@ Over the deploy limit, deliveries stay queued and retry after the hour rolls ove | Handler runs in one chain | Per account holding the subscription | 12 | 4 | - A write made through a handler's `user` in the events worker, or through a token `user` creates, is one run deeper than the event that ran the handler. Writes from anywhere else start a new chain. `user.workers.create` is refused rather than counted — a handler cannot start an untracked chain through a worker of its own. -- When an event reaches the limit, the events worker runs no handler for it. A `broadcast` one still reaches connected clients without running the persistent handler; a `single` one is still offered to a connected client first and runs there. The dropped run leaves no gap marker and doesn't count as a handler failure. +- When an event reaches the limit, the events worker runs no handler for it. Without `queue`, it still reaches connected clients without running the persistent handler. A queued one is still offered to a connected client first and runs there. The dropped run leaves no gap marker and doesn't count as a handler failure. - If the holder's plan can't be looked up, the free number applies. A server with no metering uses the paid number. #### Suspended subscriptions @@ -580,7 +580,7 @@ Deliveries are billed to the account holding the subscription: | `events:delivery:broadcast` | 10 µ¢ ($0.10 per million) | Delivered event | | `events:delivery:single` | 100 µ¢ ($1 per million) | Delivered event | -µ¢ is a microcent (a millionth of a cent). Session subscriptions bill at the broadcast rate. Handler runs bill separately as worker usage. +µ¢ is a microcent (a millionth of a cent). Deliveries for `queue` subscriptions bill on the `single` line, and every other delivery, session subscriptions included, bills at the broadcast rate. Handler runs bill separately as worker usage. Free: idle subscriptions, events a filter excluded, writes merged by coalescing, deliveries stopped by a permission check, deliveries with no client connected to receive them and no handler run, and gap markers. diff --git a/src/docs/src/recipes/build-a-group-chat.md b/src/docs/src/recipes/build-a-group-chat.md index 9f0dd1b1f..353de7d06 100644 --- a/src/docs/src/recipes/build-a-group-chat.md +++ b/src/docs/src/recipes/build-a-group-chat.md @@ -141,7 +141,8 @@ const older = await room.latest('msg:*', { cursor: page.cursor }); ## Get New Messages Live Subscribe to the room's messages with the helper's `watch()`, which uses -[`puter.events.onLocal()`](/Events/onLocal/): +[`puter.events.onLocal()`](/Events/onLocal/). New messages only need to reach +people with the chat open, so a subscription that ends with the page is enough: ```js await room.watch('msg:*', (event) => { diff --git a/src/docs/src/recipes/catch-up-on-notifications.md b/src/docs/src/recipes/catch-up-on-notifications.md index 2f16c9c84..53b7b91e9 100644 --- a/src/docs/src/recipes/catch-up-on-notifications.md +++ b/src/docs/src/recipes/catch-up-on-notifications.md @@ -23,7 +23,8 @@ as a toast or a row in a panel. ## Show New Notifications To show notifications as they arrive, subscribe to `notif:app-user` with -[`puter.events.onLocal()`](/Events/onLocal/): +[`puter.events.onLocal()`](/Events/onLocal/). It only updates the open page, and +the catch-up below covers anything missed while the page was closed: ```js const sub = await puter.events.onLocal('notif:app-user', ({ event }) => { diff --git a/src/docs/src/recipes/keep-an-open-file-in-sync.md b/src/docs/src/recipes/keep-an-open-file-in-sync.md index 683ce1150..72c9e707d 100644 --- a/src/docs/src/recipes/keep-an-open-file-in-sync.md +++ b/src/docs/src/recipes/keep-an-open-file-in-sync.md @@ -12,7 +12,8 @@ somewhere else: edit it in another tab or on their phone, rename it on the Puter desktop, or move it to the Trash. Desktop code editors handle this with a "file changed on disk" message. This recipe builds the same thing with [`puter.events.onLocal()`](/Events/onLocal/), which calls a function every time -the file changes. +the file changes. The editor only cares while the file is open, so a +subscription that ends with the page is the right fit. It builds on [Watch for Changes](/recipes/watch-for-changes/), which covers gaps, reconnects and cost. The examples use a few functions you'd write diff --git a/src/docs/src/recipes/run-code-in-the-background.md b/src/docs/src/recipes/run-code-in-the-background.md index 9dd72551c..bc2c97f36 100644 --- a/src/docs/src/recipes/run-code-in-the-background.md +++ b/src/docs/src/recipes/run-code-in-the-background.md @@ -154,7 +154,8 @@ async function startBackgroundWork () { ``` Call `startBackgroundWork()` when the app opens, after the user has given -permission. A few details: +permission. This uses the default mode, which runs the handler for every change +even while the app is closed, and tries each event once. A few details: - An `fs:` subject matches changes anywhere inside the folder. - The [`mkdir()`](/FS/mkdir/) call creates `Inbox` if it's missing, and returns @@ -244,28 +245,27 @@ the free plan and 12 on a paid plan, and doesn't report an error. ## Make Sure Every File Gets Handled -By default, the handler runs once per event. If it throws or takes longer than -30 seconds, that event is lost. There's also a limit of 60 runs a minute per +By default, the events worker runs the handler once per event. If it throws or +takes longer than 30 seconds, that event is lost. There's also a limit of 60 runs a minute per user and app, and runs past it are skipped, so if someone uploads 100 files at once, only about 60 get recorded. -To handle every file, use `delivery: 'single'`. Puter then keeps each event -until a run finishes without throwing, retrying after 2 seconds, then 4, then -8, up to 5 minutes apart. Events over the per-minute limit are delivered later -instead of being skipped: +To handle every file, use `queue: true`. Puter then treats the subscription +like a work queue: an event whose run never finishes, or fails with a network +error, timeout or other temporary error, is retried after 2 seconds, then 4, +then 8, up to 5 minutes apart. Events over the per-minute limit are delivered +later instead of being skipped: ```js -options.delivery = 'single'; -options.targets = ['worker']; +options.queue = true; ``` -Setting `targets: ['worker']` runs the handler in the events worker right away. -Without it, if the user has your app open, Puter tries those tabs first, which -can delay each event by a minute or two. +A tab of your app gets the event first only if it subscribed with a function +`handler`. Otherwise the events worker runs it right away. Subscriptions keep the delivery they were created with. To replace the subscriptions users created before this change, `startBackgroundWork()` also -checks `existing.delivery`: +checks `existing.queue`: ```js async function startBackgroundWork () { @@ -275,7 +275,7 @@ async function startBackgroundWork () { const working = existing && existing.anchor.uid === folder.uid && existing.suspendedReason !== 'permission_revoked' - && existing.delivery === 'single'; + && existing.queue; if (working) return; if (existing) await puter.events.unsubscribe(existing.subId); @@ -283,22 +283,19 @@ async function startBackgroundWork () { } ``` -Some things to know about `single`: +Some things to know about `queue`: -- **The same event can run twice.** For example, if a run finishes its work but - times out, Puter tries again with the same `event.id`. Saving the same - key-value record twice leaves the same data, but posting to the chat twice - sends two messages. To prevent most of these, save `event.id` after posting - and check for it before posting. A run that crashes between posting and - saving can still post twice. -- **Stop retrying events that can't succeed.** If retrying won't help, for - example with a file type you don't support, throw an error with - `terminal: true`. Puter drops the event instead of retrying it. -- **Five failures in a row pause the subscription**, including events you - stopped with `terminal: true`. Puter notifies you as the developer. Fix the +- **A retried event has the same `event.id`.** If a run does its work but + times out before finishing, the retry repeats it. For work like posting a + message, save `event.id` afterwards and skip IDs you've already handled. +- **A thrown error isn't retried.** The event is dropped. To have it tried + again, throw it with `retry: true`, for example when a service you call is + busy. +- **Five failures in a row pause the subscription**, retried and dropped + events alike. Puter notifies you as the developer. Fix the handler and publish it again. The subscription resumes and delivers the events it saved while paused. -- Events with `single` delivery cost the user $1 per million instead of $0.10. +- Events delivered with `queue` cost the user $1 per million instead of $0.10. See [Rate Limits and Quotas](/rate-limits-and-quotas/#events). Here's `recordUpload()` with the duplicate check. The marker key expires after @@ -360,11 +357,11 @@ A paused (*suspended*) subscription still exists, and | `suspendedReason` | What happened | What to do | | --- | --- | --- | | `handler_not_found` | The handler was removed. | Publish it again. The subscription resumes. | -| `failures` | Five runs in a row failed (`single` only). | Fix the handler and publish it again. The subscription resumes. | +| `failures` | Five runs in a row failed (`queue` only). | Fix the handler and publish it again. The subscription resumes. | | `no_credit` | The user ran out of credit. | Nothing. It resumes when they add credit. | | `permission_revoked` | The user took back `events:background`, or lost access to the folder. | Ask for the permission again, then call `startBackgroundWork()`. | -A paused `single` subscription saves up to 100 missed events and delivers them +A paused `queue` subscription saves up to 100 missed events and delivers them when it resumes. They're kept for 24 hours, or 1 hour for `no_credit`. Default subscriptions don't save missed events while paused. @@ -419,8 +416,7 @@ const inbox = `~/AppData/${puter.appID}/Inbox`; const options = { subject: `fs:${inbox}`, handlerName: 'recordUpload', - delivery: 'single', - targets: ['worker'], + queue: true, context: { webhookUrl: settings.webhookUrl }, }; @@ -437,7 +433,7 @@ async function startBackgroundWork () { const working = existing && existing.anchor.uid === folder.uid && existing.suspendedReason !== 'permission_revoked' - && existing.delivery === 'single'; + && existing.queue; if (working) return; if (existing) await puter.events.unsubscribe(existing.subId); diff --git a/src/docs/src/recipes/send-live-updates-to-other-users.md b/src/docs/src/recipes/send-live-updates-to-other-users.md index 559afda62..8bf5c94ae 100644 --- a/src/docs/src/recipes/send-live-updates-to-other-users.md +++ b/src/docs/src/recipes/send-live-updates-to-other-users.md @@ -54,7 +54,8 @@ await puter.kv.set(`${prefix}score`, { home: 1, away: 0 }); The fan finds the share with [`puter.kv.listShared()`](/KV/listShared/) and subscribes to `kv::*` with [`puter.events.onLocal()`](/Events/onLocal/). -Pass [`includeValue: true`](/Events/subjects/#getting-the-new-value) to get the new value with each event: +The fan only needs updates while their page is open, so nothing has to run in +the background. Pass [`includeValue: true`](/Events/subjects/#getting-the-new-value) to get the new value with each event: ```js const { items } = await puter.kv.listShared(); diff --git a/src/docs/src/recipes/share-a-list-with-someone.md b/src/docs/src/recipes/share-a-list-with-someone.md index 4ce6ff2f9..aa17aef9d 100644 --- a/src/docs/src/recipes/share-a-list-with-someone.md +++ b/src/docs/src/recipes/share-a-list-with-someone.md @@ -178,7 +178,8 @@ described in [Key-value data](/Events/subjects/#key-value-data). The `watch()` part of the helper calls [`puter.events.onLocal()`](/Events/onLocal/), so each person sees the other's -changes as they happen: +changes as they happen while the list is open. The list itself is saved, so +nothing is lost when nobody is watching: ```js await alicesList.watch('item:*', (event) => { diff --git a/src/docs/src/recipes/watch-for-changes.md b/src/docs/src/recipes/watch-for-changes.md index 4e7e499c1..003be6bf2 100644 --- a/src/docs/src/recipes/watch-for-changes.md +++ b/src/docs/src/recipes/watch-for-changes.md @@ -21,7 +21,9 @@ the handler. It also runs for changes made somewhere else: - Another person writes to a folder shared with the user. To subscribe, use the [`puter.events.onLocal()`](/Events/onLocal/) method. The -subscription lasts until you end it or the user closes the tab. +subscription lasts until you end it or the user closes the tab, which is what a +page that only updates what it shows needs. To run code while your app is +closed, see [Run Code in the Background](/recipes/run-code-in-the-background/). ## Watch a Key diff --git a/src/puter-js/src/modules/events/events.test.js b/src/puter-js/src/modules/events/events.test.js index b2ed0a04d..0f39d6461 100644 --- a/src/puter-js/src/modules/events/events.test.js +++ b/src/puter-js/src/modules/events/events.test.js @@ -942,7 +942,7 @@ describe('persistent delivery routing', () => { expect(sockets[0].sent.filter(s => s.verb === 'events.ack')).toEqual([]); }); - it('runs nothing for a delivery the server marked as handled by the worker', async () => { + it('runs nothing for a delivery the server marked as past the handler-chain limit', async () => { const events = makeModule(); const seen = []; events.channel.registerDurable('app-1#sub', ({ event }) => seen.push(event)); @@ -1019,7 +1019,7 @@ describe('persistent delivery routing', () => { ]); }); - it('acknowledges nothing when the handler throws, so it is delivered again', async () => { + it('declines as failed and not retryable, and acknowledges nothing, when the handler throws', async () => { const events = makeModule(); const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); events.channel.registerDurable('app-1#sub', async () => { @@ -1031,15 +1031,98 @@ describe('persistent delivery routing', () => { durable('app-1#sub', '/user/a/one.txt', { ackRequired: true, ackId: 'entry-3', + origin: 'west', }), ); await Promise.resolve(); await Promise.resolve(); expect(sockets[0].sent.filter(s => s.verb === 'events.ack')).toEqual([]); + expect(sockets[0].sent.filter(s => s.verb === 'events.nack')).toMatchObject([ + { + payload: { + subId: 'app-1#sub', + id: 'entry-3', + reason: 'failed', + retry: false, + origin: 'west', + }, + }, + ]); expect(errors).toHaveBeenCalled(); }); + it('declines a handler that throws synchronously, but not one that acked first', async () => { + const events = makeModule(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + events.channel.registerDurable('app-1#sync', () => { + throw new Error('handler bug'); + }); + events.channel.registerDurable('app-1#late', async ({ ack }) => { + await ack(); + throw new Error('after ack'); + }); + + sockets[0].fire( + 'events.delivery', + durable('app-1#sync', '/user/a/one.txt', { ackRequired: true, ackId: 'entry-s' }), + ); + sockets[0].fire( + 'events.delivery', + durable('app-1#late', '/user/a/two.txt', { ackRequired: true, ackId: 'entry-l' }), + ); + for ( let i = 0; i < 5; i++ ) await Promise.resolve(); + + expect(sockets[0].sent.filter(s => s.verb === 'events.nack').map(s => s.payload.id)) + .toEqual(['entry-s']); + expect(sockets[0].sent.filter(s => s.verb === 'events.ack').map(s => s.payload.id)) + .toEqual(['entry-l']); + }); + + it('declines a retryable failure as one to try again', async () => { + const events = makeModule(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + events.channel.registerDurable('app-1#net', async () => { + throw new TypeError('Failed to fetch'); + }); + events.channel.registerDurable('app-1#later', async () => { + throw Object.assign(new Error('try later'), { retry: true }); + }); + + for ( const [subId, ackId] of [['app-1#net', 'entry-n'], ['app-1#later', 'entry-r']] ) { + sockets[0].fire( + 'events.delivery', + durable(subId, '/user/a/one.txt', { ackRequired: true, ackId }), + ); + } + for ( let i = 0; i < 5; i++ ) await Promise.resolve(); + + expect(sockets[0].sent.filter(s => s.verb === 'events.nack').map(s => s.payload)) + .toMatchObject([ + { id: 'entry-n', reason: 'failed', retry: true }, + { id: 'entry-r', reason: 'failed', retry: true }, + ]); + }); + + it('declines a queued delivery it has no handler for, at once', async () => { + const events = makeModule(); + // Opens the connection without registering anything for `app-1#sub`. + events.channel.registerDurable('app-1#other', () => {}); + + sockets[0].fire( + 'events.delivery', + durable('app-1#sub', '/user/a/one.txt', { ackRequired: true, ackId: 'entry-4' }), + ); + // A copy everyone gets is not declined: nothing is waiting on it. + sockets[0].fire('events.delivery', durable('app-1#sub', '/user/a/two.txt')); + await Promise.resolve(); + + expect(sockets[0].sent.filter(s => s.verb === 'events.nack')).toMatchObject([ + { payload: { subId: 'app-1#sub', id: 'entry-4', reason: 'no_handler', retry: false } }, + ]); + expect(sockets[0].sent.filter(s => s.verb === 'events.ack')).toEqual([]); + }); + it('keeps routing across a reconnect without subscribing again', async () => { const events = makeModule(); const seen = []; diff --git a/src/puter-js/src/modules/events/lib/channel.js b/src/puter-js/src/modules/events/lib/channel.js index fb7f587b5..80d1a75a8 100644 --- a/src/puter-js/src/modules/events/lib/channel.js +++ b/src/puter-js/src/modules/events/lib/channel.js @@ -2,6 +2,7 @@ import { io } from 'socket.io-client'; import { PuterJSError } from '../../../lib/PuterJSError.js'; import { socketAutoUnref } from '../../../lib/socketOptions.js'; import { uuidv4 } from '../../../lib/utils.js'; +import { isRetryableError } from './retryable.js'; import { EventSubscription } from './subscription.js'; /** @typedef {import('../types.js').EventGapMarker} EventGapMarker */ @@ -35,11 +36,12 @@ const subscribePayload = (sub) => ({ ...(sub.includeValue ? { includeValue: true } : {}), }); -// The wire, fixed by the server: three verbs answered with an ack, one channel +// The wire, fixed by the server: four verbs answered with an ack, one channel // events arrive on. const SUBSCRIBE_VERB = 'events.subscribe'; const UNSUBSCRIBE_VERB = 'events.unsubscribe'; const ACK_VERB = 'events.ack'; +const NACK_VERB = 'events.nack'; const DELIVERY_CHANNEL = 'events.delivery'; const ENDED_CHANNEL = 'events.ended'; @@ -501,7 +503,7 @@ export class EventChannel { /** * @internal - * @param {{ subId?: string, event?: unknown, ackRequired?: boolean, ackId?: string, skipHandler?: boolean }} envelope + * @param {{ subId?: string, event?: unknown, ackRequired?: boolean, ackId?: string, origin?: string, skipHandler?: boolean }} envelope * @returns {void} */ route (envelope) { @@ -516,9 +518,15 @@ export class EventChannel { return; } const registered = this.durable.get(subId); - // An event for something this client has already unsubscribed from: - // in flight when `off()` was called, and no longer anybody's. - if ( registered ) this.runDurable(registered, envelope); + if ( registered ) { + this.runDurable(registered, envelope); + return; + } + // A queued delivery this client has no handler for goes back at once, + // so the events worker takes it instead of waiting out the lease. + if ( envelope.ackRequired && typeof envelope.ackId === 'string' ) { + this.nack(subId, envelope.ackId, envelope.origin, 'no_handler', false); + } } /** @@ -539,14 +547,14 @@ export class EventChannel { /** * Run a persistent subscription's handler on one delivery, unless the - * server marked it `skipHandler`: the app's events worker takes it, or it - * is past the handler-chain limit. + * server marked it `skipHandler` because it is past the handler-chain + * limit. * * The handler is handed the same environment its published copy gets in the * app's events worker, so one body runs unchanged in either place. A - * delivery owed to exactly one consumer carries `ack`: calling it settles - * the delivery, resolving without calling it settles it anyway, and - * throwing settles nothing — the lease lapses and it is delivered again. + * queued delivery is acknowledged when the handler resolves (`ack` only + * releases it early), and a throw hands it back as failed: retried if + * `isRetryableError` says so, dropped otherwise. * * @internal * @param {DurableRegistration} registration @@ -554,7 +562,7 @@ export class EventChannel { * @returns {void} */ runDurable (registration, envelope) { - // `single` always carries `ackRequired` here and must still run — + // A queued delivery always carries `ackRequired` and must still run — // only an unacknowledged broadcast copy is ever marked this way. if ( envelope.skipHandler === true && ! envelope.ackRequired ) return; @@ -585,12 +593,23 @@ export class EventChannel { settleHandler( () => registration.handler({ ...delivery, ack }), () => ack(), + error => { + if ( acked ) return; + acked = true; + this.nack( + registration.subId, + /** @type {string} */ (envelope.ackId), + envelope.origin, + 'failed', + isRetryableError(error), + ); + }, ); } /** * Tell the server a delivery was taken. Best effort: a lost ack is a - * redelivery, which `single` callers are told to expect, and there is + * redelivery, which `queue` callers are told to expect, and there is * nothing a failure here leaves for the caller to fix. * * @internal @@ -613,6 +632,31 @@ export class EventChannel { } } + /** + * Hand a queued delivery back so the server gives it to the events worker + * now. Best effort: a lost decline costs only the wait for the lease. + * + * @internal + * @param {string} subId + * @param {string} ackId + * @param {string | undefined} origin Echoed back untouched, as for `ack`. + * @param {'no_handler' | 'failed'} reason `failed` counts as a handler + * failure; `no_handler` does not. + * @param {boolean} retry Whether a failure is worth trying again. + * @returns {Promise} + */ + async nack (subId, ackId, origin, reason, retry) { + try { + await this.request( + NACK_VERB, + { subId, id: ackId, reason, retry, ...(origin ? { origin } : {}) }, + DEFAULT_TIMEOUT_MS, + ); + } catch (error) { + console.warn('[puter.events] could not decline a delivery', error); + } + } + /** * The connection is not coming back for now: fail what is waiting on it * and end every session subscription. A persistent registration is only @@ -734,24 +778,26 @@ const timeoutFor = (sub) => /** * Run a handler and, if it finishes without throwing, do whatever the delivery * still needs. A handler that throws is the app's bug: it is reported, and - * nothing is settled on its behalf. + * `onFailed` hands the delivery back. * * @param {() => unknown} run * @param {() => unknown} [onResolved] + * @param {(error: unknown) => unknown} [onFailed] * @returns {void} */ -const settleHandler = (run, onResolved) => { +const settleHandler = (run, onResolved, onFailed) => { + const failed = error => { + console.error('[puter.events] subscription handler failed', error); + onFailed?.(error); + }; try { const result = run(); if ( result instanceof Promise ) { - result.then( - () => onResolved?.(), - error => console.error('[puter.events] subscription handler failed', error), - ); + result.then(() => onResolved?.(), failed); return; } onResolved?.(); } catch (error) { - console.error('[puter.events] subscription handler failed', error); + failed(error); } }; diff --git a/src/puter-js/src/modules/events/lib/retryable.js b/src/puter-js/src/modules/events/lib/retryable.js new file mode 100644 index 000000000..bf1cf24e4 --- /dev/null +++ b/src/puter-js/src/modules/events/lib/retryable.js @@ -0,0 +1,37 @@ +/** + * Whether a handler failure is worth trying again. Anything else a handler + * throws drops the event. Shared by this client and the events worker runtime, + * which inlines this file, so it has no imports. + * + * Retryable: `retry: true` on the error; a 429 or 5xx `status`; a network, + * timeout or rate-limit error code; a `TimeoutError`; or a `TypeError` from a + * failed `fetch`. `terminal: true` always drops. + * + * @param {unknown} error What the handler threw. + * @returns {boolean} + */ +export const isRetryableError = (error) => { + if ( !error || typeof error !== 'object' ) return false; + const err = /** @type {Record} */ (error); + if ( err.terminal === true || err.code === 'events_terminal' ) return false; + if ( err.retry === true ) return true; + + const status = Number(err.status ?? err.statusCode); + if ( status === 429 || (status >= 500 && status < 600) ) return true; + if ( typeof err.code === 'string' && RETRYABLE_CODES.includes(err.code) ) return true; + if ( err.name === 'TimeoutError' ) return true; + return err.name === 'TypeError' && + typeof err.message === 'string' && + /fetch|network/i.test(err.message); +}; + +const RETRYABLE_CODES = [ + 'network_error', + 'request_timeout', + 'too_many_requests', + 'rate_limited', + 'events_connection_failed', + 'ECONNRESET', + 'ECONNREFUSED', + 'ETIMEDOUT', +]; diff --git a/src/puter-js/src/modules/events/lib/retryable.test.js b/src/puter-js/src/modules/events/lib/retryable.test.js new file mode 100644 index 000000000..bfaa51775 --- /dev/null +++ b/src/puter-js/src/modules/events/lib/retryable.test.js @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest'; +import { isRetryableError } from './retryable.js'; + +const withFields = (fields, ErrorType = Error) => + Object.assign(new ErrorType(fields.message ?? 'boom'), fields); + +describe('which handler failures are retried', () => { + it.each([ + ['an error marked retry: true', withFields({ retry: true })], + ['a 429', withFields({ status: 429 })], + ['a 503', withFields({ status: 503 })], + ['a 500 as statusCode', withFields({ statusCode: 500 })], + ['a puter.js network error', withFields({ code: 'network_error' })], + ['a puter.js request timeout', withFields({ code: 'request_timeout' })], + ['a rate limit', withFields({ code: 'too_many_requests' })], + ['a dropped events connection', withFields({ code: 'events_connection_failed' })], + ['a reset socket', withFields({ code: 'ECONNRESET' })], + ['an abort signal timeout', withFields({ name: 'TimeoutError' })], + ['a browser fetch failure', new TypeError('Failed to fetch')], + ['a node fetch failure', new TypeError('fetch failed')], + ['a fetch network error', new TypeError('NetworkError when attempting to fetch resource.')], + ])('retries %s', (_label, error) => { + expect(isRetryableError(error)).toBe(true); + }); + + it.each([ + ['a plain error', new Error('bad input')], + ['a 400', withFields({ status: 400 })], + ['a 404', withFields({ status: 404 })], + ['an unrelated TypeError', new TypeError('x is not a function')], + ['an unknown code', withFields({ code: 'subject_does_not_exist' })], + ['a thrown string', 'nope'], + ['undefined', undefined], + ['terminal: true, even on a 503', withFields({ terminal: true, status: 503 })], + ['events_terminal, even with retry: true', withFields({ code: 'events_terminal', retry: true })], + ])('drops %s', (_label, error) => { + expect(isRetryableError(error)).toBe(false); + }); +}); diff --git a/src/puter-js/src/modules/events/onPersistent.js b/src/puter-js/src/modules/events/onPersistent.js index 23a2c2818..4d6dc7b74 100644 --- a/src/puter-js/src/modules/events/onPersistent.js +++ b/src/puter-js/src/modules/events/onPersistent.js @@ -12,15 +12,14 @@ import { resolveSubject } from './lib/resolveSubject.js'; * * Unlike `onLocal()`, the subscription is stored against the account, keeps * matching while the app is closed, and is ended by - * `puter.events.unsubscribe()` rather than by navigating away. By default - * (`broadcast`, targeting the worker) the app's published handler, named by - * `handlerName`, is the one that runs — whether or not this client is open. + * `puter.events.unsubscribe()` rather than by navigating away. Each event runs + * the app's published handler, named by `handlerName`, in its events worker, + * and is also sent to every connected client. With `queue`, each event goes + * to exactly one consumer instead. * - * Passing a function as `handler` also runs it in this client, on the - * deliveries the server leaves to clients (see - * [Where the handler runs](/Events/onPersistent/#where-the-handler-runs)): - * the same body, the same `{ event, ctx }`, plus `user`, `fetch` and — for a - * subscription owed to one consumer — `ack`. + * Passing a function as `handler` also runs it in this client for each + * delivery it receives: the same body, the same `{ event, ctx }`, plus `user`, + * `fetch` and — for a `queue` subscription — `ack`. * * `context` is evaluated **here, now** — serialized once and delivered to every * invocation as a frozen `ctx`. It never re-evaluates, so a value read from the @@ -46,12 +45,11 @@ export async function onPersistent (options = {}) { 'events_handler_name_required', ); } - // A `single` delivery is owed to exactly one consumer, and the handler is - // the only one always there to take it — checked here rather than only - // discovered on the round trip. - if ( options.delivery === 'single' && ! handlerName ) { + // A queued delivery is owed to exactly one consumer, and the handler is + // the only one always there to take it. + if ( (options.queue === true || options.delivery === 'single') && ! handlerName ) { throw new PuterJSError( - 'A `single` subscription needs a `handlerName`', + 'A `queue` subscription needs a `handlerName`', 'events_handler_required', ); } @@ -62,8 +60,8 @@ export async function onPersistent (options = {}) { const body = { subject, + ...(typeof options.queue === 'boolean' ? { queue: options.queue } : {}), ...(options.delivery ? { delivery: options.delivery } : {}), - ...(options.targets ? { targets: options.targets } : {}), ...(handlerName ? { handlerName } : {}), ...(inline ? { handlerHash: inline.hash } : {}), ...(options.expiresAt !== undefined && options.expiresAt !== null diff --git a/src/puter-js/src/modules/events/persistent.test.js b/src/puter-js/src/modules/events/persistent.test.js index 5b1fb9d27..9079e9680 100644 --- a/src/puter-js/src/modules/events/persistent.test.js +++ b/src/puter-js/src/modules/events/persistent.test.js @@ -76,11 +76,10 @@ describe('onPersistent', () => { expect(sub).toBe(view); }); - it('carries delivery, targets, handlerName, expiry and includeValue when given', async () => { + it('carries queue, handlerName, expiry and includeValue when given', async () => { await makeModule().onPersistent({ subject: SUBJECT, - delivery: 'single', - targets: ['worker'], + queue: true, handlerName: 'ingestUpload', expiresAt: 4102444800, includeValue: true, @@ -88,14 +87,28 @@ describe('onPersistent', () => { expect(bodyOf()).toEqual({ subject: SUBJECT, - delivery: 'single', - targets: ['worker'], + queue: true, handlerName: 'ingestUpload', expiresAt: 4102444800, includeValue: true, }); }); + it('still passes the older `delivery` option and drops `targets`', async () => { + await makeModule().onPersistent({ + subject: SUBJECT, + delivery: 'single', + targets: ['worker'], + handlerName: 'ingestUpload', + }); + + expect(bodyOf()).toEqual({ + subject: SUBJECT, + delivery: 'single', + handlerName: 'ingestUpload', + }); + }); + it('sends an inline handler as a hash, never as source', async () => { await makeModule().onPersistent({ subject: SUBJECT, @@ -146,12 +159,13 @@ describe('onPersistent', () => { expect(bodyOf().handlerHash).toMatch(/^[0-9a-f]{64}$/); }); - it('refuses a `single` delivery with no handlerName to match it against', async () => { - const error = await rejects(() => - makeModule().onPersistent({ subject: SUBJECT, delivery: 'single' }), - ); - - expect(error.code).toBe('events_handler_required'); + it('refuses a `queue` subscription with no handlerName to match it against', async () => { + for ( const options of [{ queue: true }, { delivery: 'single' }] ) { + const error = await rejects(() => + makeModule().onPersistent({ subject: SUBJECT, ...options }), + ); + expect(error.code).toBe('events_handler_required'); + } expect(mockRequest).not.toHaveBeenCalled(); }); diff --git a/src/puter-js/src/modules/events/types.js b/src/puter-js/src/modules/events/types.js index 21338211d..16862d5cf 100644 --- a/src/puter-js/src/modules/events/types.js +++ b/src/puter-js/src/modules/events/types.js @@ -157,10 +157,10 @@ * subscription — the ambient one when the handler runs in a client. * @property {(input: unknown, init?: unknown) => Promise} [fetch] * `puter.net.fetch` where it exists, the environment's `fetch` otherwise. - * @property {() => Promise} [ack] Settles the delivery. Present only on a - * `single` subscription: calling it hands the delivery back as taken, - * returning without calling it does the same, and throwing does neither — the - * delivery is offered again when its lease lapses. + * @property {() => Promise} [ack] Optional. Marks a `queue` delivery done + * before the handler finishes, so a later throw does not undo it. Resolving + * marks it done anyway; a throw drops it unless it is retryable (a network, + * timeout, rate-limit or 5xx error, or one thrown with `retry: true`). */ /** @@ -196,20 +196,20 @@ * @typedef {Object} OnPersistentOptions * @property {string} subject What to watch — the same grammar `onLocal()` * takes, e.g. `fs:~/Documents` or `fs:~/inbox/*.json:add`. - * @property {'broadcast' | 'single'} [delivery] `broadcast` (the default) - * delivers to everything listening; `single` delivers to exactly one - * consumer, which must acknowledge, and requires a `handlerName`. - * @property {Array<'socket' | 'worker' | 'push'>} [targets] Transports the - * deliveries may take. Defaults to `['socket', 'worker']` when an app - * subscribes and `['socket']` when an account session with no app does. A - * `single` subscription may not target `push`. + * @property {boolean} [queue] Work-queue delivery: each event goes to one + * consumer at a time and is offered again until acknowledged. Requires a + * `handlerName`. Without it, every event runs the app's published handler + * in its events worker and also reaches every connected client, once each. + * @property {'broadcast' | 'single'} [delivery] Deprecated: use `queue`. + * `'single'` is the same as `queue: true`. + * @property {unknown[]} [targets] Deprecated and ignored. * @property {string} [handlerName] The published handler this subscription - * binds to. Required for `single`. + * binds to. Required for `queue`. * @property {Function | string | { file: string }} [handler] The handler * source this subscription was written against. Sent as a hash, not as * source: the subscription binds only if it matches what is published under * `handlerName`, which is also required when this is given. A function also - * runs in this client, on the deliveries the server leaves to clients. + * runs in this client for each delivery it receives. * @property {Record} [context] Values the handler needs, * evaluated **now** and delivered as a frozen `ctx` on every invocation. * Capped at 4 KB serialized. @@ -248,9 +248,8 @@ * covers everything under it. * @property {string | null} op The single operation it is limited to, or * `null` for all of them. - * @property {Array<'socket' | 'worker' | 'push'>} targets Transports its - * deliveries may take. - * @property {'broadcast' | 'single'} delivery Its delivery class. + * @property {boolean} queue Whether each event goes to one consumer at a + * time and is offered again until acknowledged. * @property {boolean} includeValue Whether its `kv:` deliveries carry the * key's new value. * @property {string | null} handlerName The handler it is bound to. diff --git a/src/puter-js/tests/api/suites/events.suite.ts b/src/puter-js/tests/api/suites/events.suite.ts index 122e27b6d..09c61e54e 100644 --- a/src/puter-js/tests/api/suites/events.suite.ts +++ b/src/puter-js/tests/api/suites/events.suite.ts @@ -101,7 +101,7 @@ const recorded = (): Array<{ path?: string; label?: unknown }> => /** * A broadcast handler, reporting whether it was handed the same environment a - * `single` handler gets — `user` and `fetch` are not conditioned on an `ack` + * `queue` handler gets — `user` and `fetch` are not conditioned on an `ack` * existing, so every consumer of a broadcast delivery gets them too. */ const BROADCAST_ENV_HANDLER = async ({ @@ -709,7 +709,7 @@ export default suite('events', { try { t.assert.ok(sub.subId, 'the subscription carries a server id'); t.assert.equal(sub.subject, `fs:${dir}`); - t.assert.equal(sub.delivery, 'broadcast'); + t.assert.equal(sub.queue, false); t.assert.equal(sub.suspendedAt, null); const held = await t.puter.events.list(); @@ -786,7 +786,7 @@ export default suite('events', { const sub = await t.puter.events.onPersistent({ subject: `fs:${dir}`, - delivery: 'single', + queue: true, handlerName: 'ingestUpload', handler: RECORDING_HANDLER, context: { label: 'ingest' }, @@ -806,7 +806,7 @@ export default suite('events', { 'the handler is handed the context the subscription carries', ); - // A `single` hands out one delivery at a time, so a second one + // A queue hands out one delivery at a time, so a second one // arriving is the acknowledgement of the first. await t.puter.fs.write(`${dir}/second.txt`, 'two'); const settled = await waitFor( @@ -843,7 +843,6 @@ export default suite('events', { const sub = await t.puter.events.onPersistent({ subject: `fs:${dir}`, - targets: ['socket'], handlerName: 'ingestEvict', handler: RECORDING_HANDLER, context: { label: 'ingest' }, @@ -886,15 +885,14 @@ export default suite('events', { const live = await open(t, `fs:${dir}`, () => {}); if (! live) return; - // Default delivery: `broadcast`, not `single` — nobody owes an - // `ack`, but the environment is the same either way. Pinned to - // `socket` so this client runs it whether or not a worker could. + // Default delivery, not `queue` — nobody owes an + // `ack`, but the environment is the same either way. This client + // runs it alongside the events worker. const sub = await t.puter.events.onPersistent({ subject: `fs:${dir}`, handlerName: 'ingestBroadcastEnv', handler: BROADCAST_ENV_HANDLER, context: { label: 'broadcast-env' }, - targets: ['socket'], }); try { await t.puter.fs.write(`${dir}/first.txt`, 'one'); @@ -925,7 +923,7 @@ export default suite('events', { t.assert.rejects(() => t.puter.events.onPersistent({ subject: `fs:${dir}`, - delivery: 'single', + queue: true, handlerName: 'ingestUpload', handler: RECORDING_HANDLER, }), @@ -937,7 +935,7 @@ export default suite('events', { const subId = await asApp(t, appUid, async () => { const sub = await t.puter.events.onPersistent({ subject: `fs:${dir}`, - delivery: 'single', + queue: true, handlerName: 'ingestUpload', handler: RECORDING_HANDLER, }); @@ -968,12 +966,11 @@ export default suite('events', { const sub = await open(t, `fs:${dir}`, () => {}); if (sub) await sub.off(); - // So is a durable one that asks for nothing else. + // So is a durable one with no handler to run. const durable = await t.puter.events.onPersistent({ subject: `fs:${dir}`, - targets: ['socket'], }); - t.assert.deepEqual(durable.targets, ['socket']); + t.assert.equal(durable.handlerName, null); await durable.off!(); }); }, diff --git a/src/worker/scripts/buildPreamble.mjs b/src/worker/scripts/buildPreamble.mjs index cb815529d..0f2c3e258 100644 --- a/src/worker/scripts/buildPreamble.mjs +++ b/src/worker/scripts/buildPreamble.mjs @@ -23,12 +23,15 @@ try { } catch { /* not in a git repo — keep "unknown" */ } const preambleVersion = `${puterJsPkg.version}+${gitSha}`; +// An included ES module loses its `export` keywords, so a file puter.js imports +// can be inlined into a preamble as plain declarations. It must have no imports. const inlineIncludes = async (filePath) => { const fileContents = await readFile(filePath, 'utf-8'); const lines = fileContents.split('\n'); const expandedLines = []; - for (const line of lines) { + for (const rawLine of lines) { + const line = rawLine.replace(/^export (const|function|class) /, '$1 '); const includeMatch = /^([ \t]*)#include "([^"]+)"$/.exec(line); if (!includeMatch) { expandedLines.push(line); diff --git a/src/worker/src/events-runtime.js b/src/worker/src/events-runtime.js index 4a18a3df3..135afef1f 100644 --- a/src/worker/src/events-runtime.js +++ b/src/worker/src/events-runtime.js @@ -13,6 +13,10 @@ * naming which one: `bad-key`, `bad-body`, `handler-broken`, `unknown-handler`, * `no-token`, `handler-threw`, `handler-terminal`. * + * A handler that throws a retryable error (see `isRetryableError`, inlined + * ahead of this file) answers 500 and is retried; any other throw answers 400 + * and drops the event. + * * An unauthorized invocation answers 500 rather than 401: the only way to get * one is a platform-side fault (a key rotated under a resident script), which * a redeploy fixes, and a 4xx would retire the delivery instead. @@ -24,6 +28,9 @@ // Captured before any handler code can run, so a handler cannot forge the // handled header by reassigning the global `Response`. const NativeResponse = Response; + // Captured for the same reason: handler code cannot replace it. + const isRetryable = + typeof isRetryableError === 'function' ? isRetryableError : () => false; const INVOKE_PATH = '/__events/invoke'; const HANDLED_HEADER = 'x-puter-events-handled'; @@ -166,7 +173,7 @@ if (!token) return answer(500, { error: 'missing delivery token' }, 'no-token'); - // `ack()` marks the delivery taken; a later throw does not unsay it. + // `ack()` releases the delivery early; a later throw does not unsay it. let acked = false; const ack = () => { acked = true; @@ -187,15 +194,13 @@ return answer(200, { ok: true }); } catch (err) { if (acked) return answer(200, { ok: true }); - const terminal = - !!err && - (err.terminal === true || err.code === 'events_terminal'); + const retry = isRetryable(err); const message = err && err.message ? String(err.message) : String(err); return answer( - terminal ? 400 : 500, + retry ? 500 : 400, { error: message }, - terminal ? 'handler-terminal' : 'handler-threw', + retry ? 'handler-threw' : 'handler-terminal', ); } }; diff --git a/src/worker/template/puter-events.template b/src/worker/template/puter-events.template index 84964e95c..c7bd02d35 100644 --- a/src/worker/template/puter-events.template +++ b/src/worker/template/puter-events.template @@ -5,4 +5,6 @@ #include "./puter-portable-core.template" +#include "../../puter-js/src/modules/events/lib/retryable.js" + #include "../src/events-runtime.js"