mirror of
https://github.com/HeyPuter/puter.git
synced 2026-10-11 14:21:51 +00:00
perf(http): skip route lifecycle work when nothing listens
The per-route lifecycle middleware built a payload, opened an event.emitAndWait span and hooked finish/close on every request, listeners or not. Each phase now checks hasListeners first. EventClient's three copies of the key-prefix walk become one memoized #matchKeys.
This commit is contained in:
1 parent
a014bec72a
commit
d841ca2842
4 files changed
+157
-39
No files matched your search
@@ -136,6 +136,30 @@ describe('EventClient', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('repeat emits', () => {
|
||||
it('reaches listeners added after the key was first emitted', () => {
|
||||
target.emit(`${key}.a.b`, {}, {});
|
||||
const wildcard = vi.fn();
|
||||
const exact = vi.fn();
|
||||
target.on(`${key}.a.*`, wildcard);
|
||||
target.on(`${key}.a.b`, exact);
|
||||
target.emit(`${key}.a.b`, {}, {});
|
||||
expect(wildcard).toHaveBeenCalledTimes(1);
|
||||
expect(exact).toHaveBeenCalledTimes(1);
|
||||
expect(target.hasListeners(`${key}.a.b`)).toBe(true);
|
||||
});
|
||||
|
||||
it('does not skip a listener when an earlier one unsubscribes mid-dispatch', () => {
|
||||
const second = vi.fn();
|
||||
const first = vi.fn(() => target.off(key, first));
|
||||
target.on(key, first);
|
||||
target.on(key, second);
|
||||
target.emit(key, {}, {});
|
||||
expect(first).toHaveBeenCalledTimes(1);
|
||||
expect(second).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('emitAndWait', () => {
|
||||
it('awaits async listeners before resolving', async () => {
|
||||
let resolved = false;
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
*/
|
||||
|
||||
import { extensionStore } from '../../extensions';
|
||||
import { BoundedTtlMap } from '../../util/boundedTtlMap.js';
|
||||
import { withSpan } from '../../util/span.js';
|
||||
import { PuterClient } from '../types';
|
||||
import {
|
||||
@@ -28,8 +29,13 @@ import {
|
||||
MatchingEvents,
|
||||
} from './types';
|
||||
|
||||
const NO_LISTENERS: readonly EventListener[] = [];
|
||||
|
||||
export class EventClient extends PuterClient {
|
||||
#eventListeners: Partial<Record<ListenKey, EventListener[]>> = {};
|
||||
#matchKeyMemo = new BoundedTtlMap<string, readonly ListenKey[]>({
|
||||
maxEntries: 1024,
|
||||
});
|
||||
|
||||
onServerStart() {
|
||||
this.emit('serverStart', {}, {});
|
||||
@@ -63,19 +69,8 @@ export class EventClient extends PuterClient {
|
||||
data: EventMap[T],
|
||||
meta: EventMetadata,
|
||||
) {
|
||||
const parts = key.split('.');
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
const matchKey = (
|
||||
i === parts.length - 1
|
||||
? key
|
||||
: `${parts.slice(0, i + 1).join('.')}.*`
|
||||
) as ListenKey;
|
||||
const extensionListeners = extensionStore.events[matchKey];
|
||||
const listeners = (this.#eventListeners[matchKey] || []).concat(
|
||||
extensionListeners || [],
|
||||
);
|
||||
if (!listeners) continue;
|
||||
for (const listener of listeners) {
|
||||
for (const matchKey of this.#matchKeys(key)) {
|
||||
for (const listener of this.#listenersFor(matchKey)) {
|
||||
this.#emitEvent(listener, key, data, meta);
|
||||
}
|
||||
}
|
||||
@@ -102,21 +97,10 @@ export class EventClient extends PuterClient {
|
||||
// Spanned because callers block on listeners — this is where time
|
||||
// spent in extension hooks (e.g. `ip.validate`) gets attributed.
|
||||
return withSpan('event.emitAndWait', { 'event.key': key }, async () => {
|
||||
const parts = key.split('.');
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
const matchKey = (
|
||||
i === parts.length - 1
|
||||
? key
|
||||
: `${parts.slice(0, i + 1).join('.')}.*`
|
||||
) as ListenKey;
|
||||
const extensionListeners = extensionStore.events[matchKey];
|
||||
const listeners = (this.#eventListeners[matchKey] || []).concat(
|
||||
extensionListeners || [],
|
||||
);
|
||||
if (!listeners) continue;
|
||||
for (const matchKey of this.#matchKeys(key)) {
|
||||
// A wildcard listener observes the event; it does not implement it.
|
||||
const isExact = matchKey === key;
|
||||
for (const listener of listeners) {
|
||||
for (const listener of this.#listenersFor(matchKey)) {
|
||||
try {
|
||||
await listener(key, data, meta);
|
||||
} catch (e) {
|
||||
@@ -142,13 +126,7 @@ export class EventClient extends PuterClient {
|
||||
* otherwise perfectly cheap and doesn't need this.
|
||||
*/
|
||||
hasListeners<T extends keyof EventMap>(key: T): boolean {
|
||||
const parts = key.split('.');
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
const matchKey = (
|
||||
i === parts.length - 1
|
||||
? key
|
||||
: `${parts.slice(0, i + 1).join('.')}.*`
|
||||
) as ListenKey;
|
||||
for (const matchKey of this.#matchKeys(key)) {
|
||||
if (this.#eventListeners[matchKey]?.length) return true;
|
||||
if (extensionStore.events[matchKey]?.length) return true;
|
||||
}
|
||||
@@ -191,6 +169,34 @@ export class EventClient extends PuterClient {
|
||||
const idx = listeners.indexOf(callback as EventListener);
|
||||
if (idx !== -1) listeners.splice(idx, 1);
|
||||
}
|
||||
/**
|
||||
* The keys a listener can be registered under to hear `key`: each
|
||||
* `<prefix>.*` shorter than the key, then the key itself. Memoized, since
|
||||
* the same few keys are emitted on every request.
|
||||
*/
|
||||
#matchKeys(key: string): readonly ListenKey[] {
|
||||
const cached = this.#matchKeyMemo.get(key);
|
||||
if (cached) return cached;
|
||||
const parts = key.split('.');
|
||||
const matchKeys = parts.map(
|
||||
(_, i) =>
|
||||
(i === parts.length - 1
|
||||
? key
|
||||
: `${parts.slice(0, i + 1).join('.')}.*`) as ListenKey,
|
||||
);
|
||||
this.#matchKeyMemo.set(key, matchKeys);
|
||||
return matchKeys;
|
||||
}
|
||||
|
||||
#listenersFor(matchKey: ListenKey): readonly EventListener[] {
|
||||
const own = this.#eventListeners[matchKey];
|
||||
const fromExtensions = extensionStore.events[matchKey];
|
||||
if (!own?.length && !fromExtensions?.length) return NO_LISTENERS;
|
||||
// A copy, so a listener that unsubscribes mid-dispatch can't make the
|
||||
// loop skip its neighbour.
|
||||
return (own ?? []).concat(fromExtensions ?? []);
|
||||
}
|
||||
|
||||
async #emitEvent<T extends keyof EventMap>(
|
||||
listener: EventListener,
|
||||
key: T,
|
||||
|
||||
@@ -19,12 +19,9 @@
|
||||
|
||||
import { EventEmitter } from 'node:events';
|
||||
import type { Request, Response } from 'express';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { EventClient } from '../../clients/event/EventClient';
|
||||
import type {
|
||||
EventMap,
|
||||
RouteLifecycleEvent,
|
||||
} from '../../clients/event/types';
|
||||
import type { EventMap, RouteLifecycleEvent } from '../../clients/event/types';
|
||||
import type { IConfig } from '../../types';
|
||||
import {
|
||||
createRouteLifecycleMiddleware,
|
||||
@@ -98,6 +95,78 @@ describe('routeEventKeyBase', () => {
|
||||
});
|
||||
|
||||
describe('createRouteLifecycleMiddleware', () => {
|
||||
it('does no event work for a route nobody listens to', async () => {
|
||||
const events = makeEvents();
|
||||
// A listener elsewhere must not make this route pay.
|
||||
record(events, 'route.get.other.before' as keyof EventMap);
|
||||
const emitAndWait = vi.spyOn(events, 'emitAndWait');
|
||||
const emit = vi.spyOn(events, 'emit');
|
||||
|
||||
const mw = createRouteLifecycleMiddleware(events, POST, PATH);
|
||||
const res = new FakeRes();
|
||||
let nextCalled = false;
|
||||
await mw(makeReq('u-1'), res as unknown as Response, () => {
|
||||
nextCalled = true;
|
||||
});
|
||||
res.writableFinished = true;
|
||||
res.emit('finish');
|
||||
res.emit('close');
|
||||
|
||||
expect(nextCalled).toBe(true);
|
||||
expect(emitAndWait).not.toHaveBeenCalled();
|
||||
expect(emit).not.toHaveBeenCalled();
|
||||
expect(res.listenerCount('finish')).toBe(0);
|
||||
expect(res.listenerCount('close')).toBe(0);
|
||||
});
|
||||
|
||||
it('skips the awaited before emit when only the terminal phase is observed', async () => {
|
||||
const events = makeEvents();
|
||||
const after = record(events, `${BASE}.after` as keyof EventMap);
|
||||
const emitAndWait = vi.spyOn(events, 'emitAndWait');
|
||||
|
||||
const mw = createRouteLifecycleMiddleware(events, POST, PATH);
|
||||
const res = new FakeRes();
|
||||
await mw(makeReq('u-1'), res as unknown as Response, () => {});
|
||||
res.writableFinished = true;
|
||||
res.emit('finish');
|
||||
|
||||
expect(emitAndWait).not.toHaveBeenCalled();
|
||||
expect(after).toHaveLength(1);
|
||||
expect(after[0]).toMatchObject({
|
||||
phase: 'after',
|
||||
actorUid: 'user:u-1',
|
||||
});
|
||||
});
|
||||
|
||||
it('skips the response hooks when only before is observed', async () => {
|
||||
const events = makeEvents();
|
||||
const before = record(events, `${BASE}.before` as keyof EventMap);
|
||||
|
||||
const mw = createRouteLifecycleMiddleware(events, POST, PATH);
|
||||
const res = new FakeRes();
|
||||
let nextCalled = false;
|
||||
await mw(makeReq(), res as unknown as Response, () => {
|
||||
nextCalled = true;
|
||||
});
|
||||
|
||||
expect(nextCalled).toBe(true);
|
||||
expect(before).toHaveLength(1);
|
||||
expect(res.listenerCount('finish')).toBe(0);
|
||||
});
|
||||
|
||||
it('fires wildcard listeners on every phase', async () => {
|
||||
const events = makeEvents();
|
||||
const seen = record(events, 'route.*' as keyof EventMap);
|
||||
|
||||
const mw = createRouteLifecycleMiddleware(events, POST, PATH);
|
||||
const res = new FakeRes();
|
||||
await mw(makeReq(), res as unknown as Response, () => {});
|
||||
res.writableFinished = true;
|
||||
res.emit('finish');
|
||||
|
||||
expect(seen.map((e) => e.phase)).toEqual(['before', 'after']);
|
||||
});
|
||||
|
||||
it('emits before, then after on a clean finish', async () => {
|
||||
const events = makeEvents();
|
||||
const before = record(events, `${BASE}.before` as keyof EventMap);
|
||||
|
||||
@@ -61,6 +61,8 @@ export const routeEventKeyBase = (
|
||||
* off the real status, not `reject`.
|
||||
*
|
||||
* Subscribers can listen on `route.*`, `route.<method>.*`, or the exact key.
|
||||
* Each phase is skipped when nothing listens for it, so an unobserved route
|
||||
* pays for a few map lookups and nothing else.
|
||||
*/
|
||||
export const createRouteLifecycleMiddleware = (
|
||||
events: EventClient,
|
||||
@@ -70,8 +72,18 @@ export const createRouteLifecycleMiddleware = (
|
||||
const pathLabel =
|
||||
typeof fullPath === 'string' ? fullPath : String(fullPath);
|
||||
const keyBase = routeEventKeyBase(method, fullPath);
|
||||
const beforeKey = `${keyBase}.before` as const;
|
||||
|
||||
return async (req: Request, res, next) => {
|
||||
const observeBefore = events.hasListeners(beforeKey);
|
||||
const observeEnd =
|
||||
events.hasListeners(`${keyBase}.after`) ||
|
||||
events.hasListeners(`${keyBase}.error`);
|
||||
if (!observeBefore && !observeEnd) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
|
||||
const actor = req.actor ? actorUid(req.actor) : undefined;
|
||||
const startedAt = Date.now();
|
||||
const base = {
|
||||
@@ -89,7 +101,9 @@ export const createRouteLifecycleMiddleware = (
|
||||
allow: true as boolean,
|
||||
rejectReason: undefined as string | undefined,
|
||||
};
|
||||
await events.emitAndWait(`${keyBase}.before`, beforeEvent, {});
|
||||
if (observeBefore) {
|
||||
await events.emitAndWait(beforeKey, beforeEvent, {});
|
||||
}
|
||||
|
||||
// Explicit veto: a listener set `allow = false`. Emit `reject` and
|
||||
// answer 403 — unless the listener already wrote its own response, in
|
||||
@@ -137,6 +151,11 @@ export const createRouteLifecycleMiddleware = (
|
||||
return;
|
||||
}
|
||||
|
||||
if (!observeEnd) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
|
||||
let settled = false;
|
||||
const settle = (aborted: boolean) => {
|
||||
if (settled) return;
|
||||
|
||||
Reference in new issue
Block a user