diff --git a/.changeset/synchronous-render-hot-lane.md b/.changeset/synchronous-render-hot-lane.md new file mode 100644 index 000000000..92084fcb4 --- /dev/null +++ b/.changeset/synchronous-render-hot-lane.md @@ -0,0 +1,5 @@ +--- +"oc": patch +--- + +Keep the warm component render path synchronous and allocation-light: cached environment lookups no longer suspend through a promise, nested-renderer and repository callback adapters are created once per registry instead of per render, and `component-retrieved` telemetry payloads are built only when a listener exists at completion. diff --git a/packages/oc/src/registry/routes/helpers/get-component-retrieving-info.ts b/packages/oc/src/registry/routes/helpers/get-component-retrieving-info.ts deleted file mode 100644 index 614e5c955..000000000 --- a/packages/oc/src/registry/routes/helpers/get-component-retrieving-info.ts +++ /dev/null @@ -1,44 +0,0 @@ -import type { IncomingHttpHeaders } from 'node:http'; - -interface Options { - headers: IncomingHttpHeaders; - name: string; - parameters: IncomingHttpHeaders; - version: string; -} - -interface EventData { - headers: IncomingHttpHeaders; - name: string; - parameters: IncomingHttpHeaders; - requestVersion: string; - duration: number; -} - -export default function getComponentRetrievingInfo(options: Options): { - extend(obj: unknown): void; - getData(): EventData; -} { - const eventData: EventData = { - headers: options.headers, - name: options.name, - parameters: options.parameters, - requestVersion: options.version || '', - duration: 0 - }; - - const start = process.hrtime(); - - return { - extend(obj: unknown) { - Object.assign(eventData, obj); - }, - getData() { - const delta = process.hrtime(start); - const nanosec = delta[0] * 1e9 + delta[1]; - eventData.duration = nanosec / 1e3; - - return eventData; - } - }; -} diff --git a/packages/oc/src/registry/routes/helpers/get-component.ts b/packages/oc/src/registry/routes/helpers/get-component.ts index 42bbbcef5..da36b9fd3 100644 --- a/packages/oc/src/registry/routes/helpers/get-component.ts +++ b/packages/oc/src/registry/routes/helpers/get-component.ts @@ -23,7 +23,6 @@ import { validateTemplateOcVersion } from '../../domain/validators'; import applyDefaultValues from './apply-default-values'; import { processStackTrace } from './format-error-stack'; import * as getComponentFallback from './get-component-fallback'; -import GetComponentRetrievingInfo from './get-component-retrieving-info'; export interface RendererOptions { action?: string; @@ -58,6 +57,7 @@ export interface GetComponentResult { message: string; stack: string; originalError: unknown; + frame?: string; }; missingPlugins?: string[]; missingDependencies?: string[]; @@ -183,29 +183,68 @@ export default function getComponent( return promise; }; - const getEnv = async ( - component: Component - ): Promise> => { + type EnvLookupCallback = (err: unknown, env?: Record) => void; + + const lookupEnv = (component: Component, cb: EnvLookupCallback): void => { const cacheKey = `${component.name}/${component.version}/.env`; const cached = cache.get>('file-contents', cacheKey); - if (cached !== undefined) return cached; + if (cached !== undefined) { + cb(null, cached); + return; + } - return singleFlight(cacheKey, async () => { + singleFlight(cacheKey, async () => { const env = component.oc.files.env ? await repository.getEnv(component.name, component.version) : {}; cache.set('file-contents', cacheKey, env); return env; - }); + }).then((env) => cb(null, env), cb); }; - const renderer = async ( + const enrichLocalErrorStack = async ( + component: Component, + err: any, + response: GetComponentResult + ): Promise => { + try { + const { content } = await repository + .getDataProvider(component.name, component.version) + .catch(() => ({ content: null })); + if (!content || !err.stack || !response.response.details) { + return; + } + + const processedStack = await processStackTrace({ + stackTrace: err.stack, + code: content + }).catch(() => null); + if (!processedStack) { + return; + } + + response.response.details.stack = processedStack.stack; + response.response.details.frame = processedStack.frame; + + console.log( + `Error rendering component ${component.name} ${component.version}` + ); + console.log(processedStack.stack); + console.log(processedStack.frame); + } catch { + // keep the original error stack when local diagnostics fail + } + }; + + const renderer = ( options: RendererOptions, cb: (result: GetComponentResult) => void - ) => { - const nestedRenderer = NestedRenderer(renderer, options.conf); - const retrievingInfo = GetComponentRetrievingInfo(options); + ): void => { + const retrievalStart = process.hrtime.bigint(); + let retrievedHref: string | undefined; + let retrievedVersion: string | undefined; + let retrievedRenderMode: string | undefined; let responseHeaders: Record | undefined; const responseCookies: Array<{ name: string; @@ -219,22 +258,46 @@ export default function getComponent( return paramOverride || options.headers['accept-language']; }; - const callback = (result: GetComponentResult) => { - if (responseCookies.length > 0 && !result.cookies) { - result.cookies = responseCookies; + const fireComponentRetrievedEvent = (result: GetComponentResult): void => { + if (!eventsHandler.hasListeners('component-retrieved')) { + return; + } + + const eventData: Record = { + headers: options.headers, + name: options.name, + parameters: options.parameters, + requestVersion: options.version || '' + }; + + if (retrievedHref !== undefined) { + eventData['href'] = retrievedHref; + eventData['version'] = retrievedVersion; + eventData['renderMode'] = retrievedRenderMode; } + if (result.response.error) { - retrievingInfo.extend(result.response); + Object.assign(eventData, result.response); } - retrievingInfo.extend({ status: result.status }); + eventData['status'] = result.status; + eventData['duration'] = + Number(process.hrtime.bigint() - retrievalStart) / 1e3; + + eventsHandler.fire('component-retrieved', eventData as any); + }; + + const callback = (result: GetComponentResult) => { + if (responseCookies.length > 0 && !result.cookies) { + result.cookies = responseCookies; + } Object.assign(result.response, { name: options.name, requestVersion: options.version || '' }); - eventsHandler.fire('component-retrieved', retrievingInfo.getData()); + fireComponentRetrievedEvent(result); return cb(result); }; @@ -247,7 +310,7 @@ export default function getComponent( parameters: options.parameters }; - fromPromise(repository.getComponent)( + getComponentCb( requestedComponent.name, requestedComponent.version, (err, component) => { @@ -389,7 +452,7 @@ export default function getComponent( return filteredHeaders; }; - const returnComponent = async (err: any, data: any) => { + const returnComponent = (err: any, data: any) => { if (componentCallbackDone) { return; } @@ -437,11 +500,9 @@ export default function getComponent( } } - retrievingInfo.extend({ - href: componentHref, - version: component.version, - renderMode - }); + retrievedHref = componentHref; + retrievedVersion = component.version; + retrievedRenderMode = renderMode; if (err || !data) { err = @@ -475,25 +536,10 @@ export default function getComponent( }; if (conf.local && err.stack) { - const { content } = await repository - .getDataProvider(component.name, component.version) - .catch(() => ({ content: null })); - if (content) { - const processedStack = await processStackTrace({ - stackTrace: err.stack, - code: content - }).catch(() => null); - if (processedStack) { - response.response.details.stack = processedStack.stack; - response.response.details.frame = processedStack.frame; - - console.log( - `Error rendering component ${component.name} ${component.version}` - ); - console.log(processedStack.stack); - console.log(processedStack.frame); - } - } + enrichLocalErrorStack(component, err, response) + .catch(() => {}) + .then(() => callback(response)); + return; } return callback(response); @@ -630,7 +676,7 @@ export default function getComponent( returnComponent(null, { component: { props } }); } else { - fromPromise(getEnv)(component, (err, env) => { + lookupEnv(component, (err, env) => { if (err) { componentCallbackDone = true; @@ -663,14 +709,14 @@ export default function getComponent( return parsedAcceptLanguage; }, baseUrl: conf.baseUrl, - env: { ...conf.env, ...env }, + env: { ...conf.env, ...(env || {}) }, params, plugins: convertPlugins({ name: component.name, version: component.version }), - renderComponent: fromPromise(nestedRenderer.renderComponent), - renderComponents: fromPromise(nestedRenderer.renderComponents), + renderComponent: nestedRenderComponent, + renderComponents: nestedRenderComponents, requestHeaders: options.headers, requestIp: options.ip, setEmptyResponse, @@ -826,5 +872,10 @@ export default function getComponent( ); }; + const nestedRenderer = NestedRenderer(renderer, conf); + const nestedRenderComponent = fromPromise(nestedRenderer.renderComponent); + const nestedRenderComponents = fromPromise(nestedRenderer.renderComponents); + const getComponentCb = fromPromise(repository.getComponent); + return renderer; } diff --git a/packages/oc/test/unit/registry-routes-helpers-get-component-timing.js b/packages/oc/test/unit/registry-routes-helpers-get-component-timing.js new file mode 100644 index 000000000..37edc2cc8 --- /dev/null +++ b/packages/oc/test/unit/registry-routes-helpers-get-component-timing.js @@ -0,0 +1,574 @@ +const Client = require('oc-client'); +const expect = require('chai').expect; +const injectr = require('injectr'); +const sinon = require('sinon'); + +describe('registry : routes : helpers : get-component timing', () => { + const mockedComponents = require('../fixtures/mocked-components'); + const simpleView = mockedComponents['simple-component'].view; + const noop = () => {}; + + const PROVIDERS = { + syncSuccess: + '"use strict";module.exports.data=function(ctx,cb){cb(null,{done:true});};', + asyncSuccess: + '"use strict";module.exports.data=function(ctx,cb){setTimeout(function(){cb(null,{done:true});},0);};', + syncThrow: + '"use strict";module.exports.data=function(){throw new Error("sync boom");};', + syncErrorCallback: + '"use strict";module.exports.data=function(ctx,cb){cb(new Error("boom"));};', + asyncErrorCallback: + '"use strict";module.exports.data=function(ctx,cb){setTimeout(function(){cb(new Error("async boom"));},0);};', + doubleCallback: + '"use strict";module.exports.data=function(ctx,cb){cb(null,{n:1});cb(null,{n:2});};', + tracedAsyncSuccess: + '"use strict";module.exports.data=function(ctx,cb){console.log("provider-invoked");setTimeout(function(){cb(null,{done:true});},0);};', + hung: + '"use strict";module.exports.data=function(ctx,cb){console.log("provider-invoked");};' + }; + + let GetComponent; + let mockedRepository; + let retrievedSpy; + let listenersActive; + + const makeComponentParams = ({ data, env = false }) => ({ + package: { + name: 'timing-component', + version: '1.0.0', + oc: { + container: false, + renderInfo: false, + files: { + template: { + type: 'jade', + hashKey: '8c1fbd954f2b0d8cd5cf11c885fed4805225749f', + src: 'template.js' + }, + dataProvider: { + type: 'node.js', + hashKey: 'timing-provider-hash', + src: 'server.js' + }, + ...(env ? { env: '.env' } : {}) + } + } + }, + data, + view: simpleView + }); + + const buildRepository = (params) => ({ + getCompiledView: sinon.stub().resolves(params.view), + getComponent: sinon.stub().resolves(params.package), + getEnv: params.package.oc.files.env + ? sinon.stub().resolves({ secret: 'secretvalue' }) + : sinon.stub().rejects(new Error('no env')), + getDataProvider: sinon + .stub() + .resolves({ content: params.data, filePath: '/path/to/server.js' }), + getTemplatesInfo: sinon.stub().returns([ + { type: 'oc-template-jade', version: '6.0.1', externals: [] } + ]), + getTemplate: (type) => + type === 'jade' || type === 'oc-template-jade' + ? require('oc-template-jade') + : undefined, + getStaticFilePath: sinon.stub().returns('//my-cdn.com/files/') + }); + + const initialise = ( + params, + { local = false, realEventsHandler = false, trace = undefined } = {} + ) => { + const providerLogLabels = []; + const sandboxConsole = { + log: (...args) => { + if (args[0] === 'provider-invoked') { + providerLogLabels.push('provider-invoked'); + if (trace) { + trace.push('provider-invoked'); + } + } + }, + error: noop, + warn: noop, + info: noop + }; + retrievedSpy = sinon.spy(); + listenersActive = true; + mockedRepository = buildRepository(params); + + const injections = { + 'oc-client': () => { + const client = Client(); + return { + renderTemplate: (template, data, renderOptions, cb) => + client.renderTemplate(template, data, renderOptions, cb) + }; + } + }; + + if (!realEventsHandler) { + injections['../../domain/events-handler'] = { + on: noop, + off: noop, + fire: (eventName, eventData) => { + if (trace) { + trace.push(`event:${eventName}`); + } + if (eventName === 'component-retrieved') { + retrievedSpy(eventName, eventData); + } + }, + hasListeners: () => listenersActive + }; + } + + GetComponent = injectr( + '../../dist/registry/routes/helpers/get-component.js', + injections, + { console: sandboxConsole, Buffer, clearTimeout, setTimeout, process } + ).default; + + return { providerLogLabels }; + }; + + const baseConf = (extra) => ({ + baseUrl: 'http://components.com/', + ...extra + }); + + const baseOptions = (extra = {}) => ({ + name: 'timing-component', + headers: {}, + parameters: {}, + version: '1.0.0', + conf: baseConf(), + ...extra + }); + + const renderVia = (getComponent, options) => + new Promise((resolve) => { + getComponent(options, resolve); + }); + + const render = (options) => renderVia(GetComponent(baseConf(), mockedRepository), options); + + const retrievedEvents = () => retrievedSpy.getCalls(); + + const completeOnce = async (options) => { + let callback; + await new Promise((resolve) => { + callback = sinon.spy(resolve); + GetComponent( + baseConf(), + mockedRepository + )(options, callback); + }); + return callback; + }; + + describe('callback ordering invariants', () => { + it('returns from the renderer call before completion and preserves provider → event → callback order', async () => { + const trace = []; + const { providerLogLabels } = initialise( + makeComponentParams({ data: PROVIDERS.tracedAsyncSuccess }), + { local: true, trace } + ); + const getComponent = GetComponent(baseConf({ local: true }), mockedRepository); + + const completion = new Promise((resolve) => { + getComponent( + baseOptions({ conf: baseConf({ local: true }) }), + (result) => { + trace.push(`callback:${result.status}`); + resolve(result); + } + ); + trace.push('renderer-return'); + queueMicrotask(() => trace.push('microtask-sentinel')); + }); + const result = await completion; + + expect(result.status).to.equal(200); + expect(trace[0]).to.equal('renderer-return'); + expect(trace).to.include('microtask-sentinel'); + const indexOf = (label) => { + const position = trace.indexOf(label); + expect(position, `trace should contain ${label}`).to.be.at.least(0); + return position; + }; + expect(providerLogLabels).to.eql(['provider-invoked']); + expect(indexOf('renderer-return')).to.be.below(indexOf('provider-invoked')); + expect(indexOf('provider-invoked')).to.be.below( + indexOf('event:component-retrieved') + ); + expect(indexOf('event:component-retrieved')).to.be.below( + indexOf('callback:200') + ); + expect(retrievedEvents()).to.have.lengthOf(1); + }); + + it('does not look up env or invoke the provider before repository resolution completes', async () => { + const params = makeComponentParams({ + data: PROVIDERS.syncSuccess, + env: true + }); + initialise(params); + let resolveRepository; + mockedRepository.getComponent = () => + new Promise((resolve) => { + resolveRepository = resolve; + }); + + const options = baseOptions({ + parameters: {}, + headers: {} + }); + const completion = render(options); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(mockedRepository.getEnv.callCount, 'env must wait').to.equal(0); + expect( + mockedRepository.getDataProvider.callCount, + 'provider must wait' + ).to.equal(0); + + resolveRepository(params.package); + const result = await completion; + + expect(result.status).to.equal(200); + expect(mockedRepository.getEnv.callCount).to.equal(1); + expect(mockedRepository.getDataProvider.callCount).to.equal(1); + }); + + it('invokes the provider only after env resolution completes', async () => { + initialise(makeComponentParams({ data: PROVIDERS.syncSuccess, env: true })); + + const result = await render(baseOptions()); + + expect(result.status).to.equal(200); + expect(mockedRepository.getComponent.callCount).to.equal(1); + expect(mockedRepository.getEnv.callCount).to.equal(1); + expect(mockedRepository.getDataProvider.callCount).to.equal(1); + sinon.assert.callOrder( + mockedRepository.getComponent, + mockedRepository.getEnv, + mockedRepository.getDataProvider + ); + }); + }); + + describe('exact-once completion invariants', () => { + it('produces exactly one error result when a provider throws synchronously', async () => { + initialise(makeComponentParams({ data: PROVIDERS.syncThrow })); + const callback = await completeOnce(baseOptions()); + + expect(callback.callCount).to.equal(1); + const result = callback.firstCall.args[0]; + expect(result.status).to.equal(500); + expect(result.response.code).to.equal('GENERIC_ERROR'); + expect(result.response.details.originalError).to.be.an('error'); + expect(result.response.details.originalError.message).to.equal( + 'sync boom' + ); + expect(retrievedEvents()).to.have.lengthOf(1); + expect(retrievedEvents()[0].args[1].status).to.equal(500); + sinon.assert.callOrder(retrievedSpy, callback); + }); + + it('produces exactly one result for an asynchronously succeeding provider', async () => { + initialise(makeComponentParams({ data: PROVIDERS.asyncSuccess })); + const callback = await completeOnce(baseOptions()); + + expect(callback.callCount).to.equal(1); + expect(callback.firstCall.args[0].status).to.equal(200); + expect(callback.firstCall.args[0].response.html).to.not.equal(undefined); + expect(retrievedEvents()).to.have.lengthOf(1); + }); + + it('produces exactly one error result for an asynchronously failing provider', async () => { + initialise(makeComponentParams({ data: PROVIDERS.asyncErrorCallback })); + const callback = await completeOnce(baseOptions()); + + expect(callback.callCount).to.equal(1); + const result = callback.firstCall.args[0]; + expect(result.status).to.equal(500); + expect(result.response.details.originalError.message).to.equal( + 'async boom' + ); + expect(retrievedEvents()).to.have.lengthOf(1); + expect(retrievedEvents()[0].args[1].status).to.equal(500); + }); + + it('completes exactly once when a provider calls back twice', async () => { + initialise(makeComponentParams({ data: PROVIDERS.doubleCallback })); + const callback = await completeOnce( + baseOptions({ + headers: { accept: 'application/vnd.oc.unrendered+json' } + }) + ); + + expect(callback.callCount).to.equal(1); + expect(callback.firstCall.args[0].status).to.equal(200); + expect(callback.firstCall.args[0].response.data).to.deep.equal({ n: 1 }); + expect(retrievedEvents()).to.have.lengthOf(1); + }); + }); + + describe('timeout invariants', () => { + it('clears the timeout before dispatching the retrieval event and the public callback', async () => { + const clock = sinon.useFakeTimers(); + try { + initialise(makeComponentParams({ data: PROVIDERS.hung })); + const getComponent = GetComponent(baseConf(), mockedRepository); + let callback; + + const completion = new Promise((resolve) => { + callback = sinon.spy(resolve); + getComponent( + baseOptions({ conf: baseConf({ executionTimeout: 1 }) }), + callback + ); + }); + + for ( + let i = 0; + i < 100 && clock.countTimers() === 0; + i++ + ) { + await Promise.resolve(); + } + expect( + clock.countTimers(), + 'execution timeout must be armed' + ).to.be.above(0); + clock.tick(1000); + + await completion; + + expect(callback.callCount).to.equal(1); + const result = callback.firstCall.args[0]; + expect(result.status).to.equal(500); + expect(result.response.error).to.contain('timeout'); + expect(clock.countTimers(), 'timeout must be cleared').to.equal(0); + expect(retrievedEvents()).to.have.lengthOf(1); + sinon.assert.callOrder(retrievedSpy, callback); + } finally { + clock.restore(); + } + }); + }); + + describe('retrieval event payload invariants', () => { + it('fires component-retrieved exactly once with final fields immediately before the public callback on success', async () => { + initialise(makeComponentParams({ data: PROVIDERS.syncSuccess })); + let callback; + + await new Promise((resolve) => { + callback = sinon.spy(resolve); + GetComponent( + baseConf(), + mockedRepository + )( + baseOptions({ + headers: { 'accept-language': 'en-GB' }, + parameters: { a: '1' }, + version: '2.3.4' + }), + callback + ); + }); + + const events = retrievedEvents(); + expect(events).to.have.lengthOf(1); + const eventData = events[0].args[1]; + expect(Object.keys(eventData).sort()).to.eql([ + 'duration', + 'headers', + 'href', + 'name', + 'parameters', + 'renderMode', + 'requestVersion', + 'status', + 'version' + ]); + expect(eventData.headers).to.eql({ 'accept-language': 'en-GB' }); + expect(eventData.name).to.equal('timing-component'); + expect(eventData.parameters).to.eql({ a: '1' }); + expect(eventData.requestVersion).to.equal('2.3.4'); + expect(eventData.href).to.equal( + 'http://components.com/timing-component/2.3.4?a=1' + ); + expect(eventData.version).to.equal('1.0.0'); + expect(eventData.renderMode).to.equal('rendered'); + expect(eventData.status).to.equal(200); + expect(eventData.duration).to.be.above(0); + expect(retrievedSpy.lastCall.callId).to.equal( + callback.firstCall.callId - 1 + ); + }); + + it('preserves response fields in the component-retrieved payload for errors', async () => { + initialise(makeComponentParams({ data: PROVIDERS.asyncErrorCallback })); + const callback = await completeOnce(baseOptions()); + + expect(callback.callCount).to.equal(1); + const events = retrievedEvents(); + expect(events).to.have.lengthOf(1); + const eventData = events[0].args[1]; + expect(Object.keys(eventData).sort()).to.eql([ + 'code', + 'details', + 'duration', + 'error', + 'headers', + 'href', + 'name', + 'parameters', + 'renderMode', + 'requestVersion', + 'status', + 'version' + ]); + expect(eventData.code).to.equal('GENERIC_ERROR'); + expect(eventData.error).to.be.a('string'); + expect(eventData.status).to.equal(500); + expect(eventData.duration).to.be.above(0); + }); + }); + + describe('local diagnostics invariants', () => { + it('finishes stack enrichment before the retrieval event and the public callback', async () => { + initialise( + makeComponentParams({ data: PROVIDERS.syncErrorCallback }), + { local: true } + ); + + const result = await render(baseOptions({ conf: baseConf({ local: true }) })); + + expect(result.status).to.equal(500); + expect(mockedRepository.getDataProvider.callCount).to.equal(2); + expect(retrievedEvents()).to.have.lengthOf(1); + sinon.assert.callOrder(mockedRepository.getDataProvider, retrievedSpy); + }); + + it('falls back to the original stack when enrichment fails', async () => { + initialise( + makeComponentParams({ data: PROVIDERS.syncErrorCallback }), + { local: true } + ); + mockedRepository.getDataProvider = sinon + .stub() + .onFirstCall() + .resolves({ + content: PROVIDERS.syncErrorCallback, + filePath: '/path/to/server.js' + }) + .onSecondCall() + .rejects(new Error('storage down')); + + let callback; + await new Promise((resolve) => { + callback = sinon.spy(resolve); + GetComponent( + baseConf({ local: true }), + mockedRepository + )(baseOptions({ conf: baseConf({ local: true }) }), callback); + }); + + expect(callback.callCount).to.equal(1); + const result = callback.firstCall.args[0]; + expect(result.status).to.equal(500); + expect(result.response.details.originalError.message).to.equal('boom'); + expect(result.response.details.stack).to.equal( + result.response.details.originalError.stack + ); + }); + }); + + describe('warm env cache invariants', () => { + it('serves consecutive renders without a second repository env lookup', async () => { + const envParams = makeComponentParams({ + data: '"use strict";module.exports.data=function(ctx,cb){cb(null,{mySecret:ctx.env.secret});};', + env: true + }); + initialise(envParams); + const getComponent = GetComponent(baseConf(), mockedRepository); + const options = baseOptions({ + headers: { accept: 'application/vnd.oc.unrendered+json' } + }); + + const first = await renderVia(getComponent, options); + const second = await renderVia(getComponent, options); + + expect(first.status).to.equal(200); + expect(second.status).to.equal(200); + expect(first.response.data).to.deep.equal({ mySecret: 'secretvalue' }); + expect(second.response.data).to.deep.equal({ mySecret: 'secretvalue' }); + expect(mockedRepository.getEnv.callCount).to.equal(1); + expect(retrievedEvents()).to.have.lengthOf(2); + }); + }); + + describe('dynamic listener semantics with the real events handler', () => { + const eventsHandler = + require('../../dist/registry/domain/events-handler').default; + let listener; + let pendingResolution; + + const setupRealHandler = (params) => { + initialise(params, { realEventsHandler: true }); + mockedRepository.getComponent = () => pendingResolution.promise; + }; + + beforeEach(() => { + eventsHandler.reset(); + pendingResolution = {}; + pendingResolution.promise = new Promise((resolve) => { + pendingResolution.resolve = resolve; + }); + listener = sinon.spy(); + }); + + afterEach(() => { + eventsHandler.reset(); + }); + + it('delivers the completion event to a listener added mid-flight', async () => { + const params = makeComponentParams({ data: PROVIDERS.syncSuccess }); + setupRealHandler(params); + + const completion = render(baseOptions()); + eventsHandler.on('component-retrieved', listener); + pendingResolution.resolve(params.package); + + const result = await completion; + + expect(result.status).to.equal(200); + expect(listener.callCount).to.equal(1); + const eventData = listener.firstCall.args[0]; + expect(eventData.name).to.equal('timing-component'); + expect(eventData.status).to.equal(200); + expect(eventData.duration).to.be.above(0); + }); + + it('does not deliver the completion event to a listener removed mid-flight', async () => { + const params = makeComponentParams({ data: PROVIDERS.syncSuccess }); + setupRealHandler(params); + eventsHandler.on('component-retrieved', listener); + + const completion = render(baseOptions()); + eventsHandler.off('component-retrieved', listener); + pendingResolution.resolve(params.package); + + const result = await completion; + + expect(result.status).to.equal(200); + expect(listener.callCount).to.equal(0); + }); + }); +}); diff --git a/packages/oc/test/unit/registry-routes-helpers-get-component.js b/packages/oc/test/unit/registry-routes-helpers-get-component.js index bf3f8bd67..558768a9e 100644 --- a/packages/oc/test/unit/registry-routes-helpers-get-component.js +++ b/packages/oc/test/unit/registry-routes-helpers-get-component.js @@ -19,7 +19,8 @@ describe('registry : routes : helpers : get-component', () => { { '../../domain/events-handler': { on: () => {}, - fire: fireStub + fire: fireStub, + hasListeners: () => true }, 'oc-client': () => { const client = Client(); @@ -33,7 +34,7 @@ describe('registry : routes : helpers : get-component', () => { }; } }, - { console, Buffer, clearTimeout, setTimeout } + { console, Buffer, clearTimeout, setTimeout, process } ).default; mockedRepository = { diff --git a/plans/README.md b/plans/README.md index 8c14a3322..dd28a778b 100644 --- a/plans/README.md +++ b/plans/README.md @@ -6,7 +6,7 @@ Generated on 2026-07-23 and extended on 2026-08-17. Each numbered plan is intend | Plan | Title | Priority | Effort | Depends on | Status | |------|-------|----------|--------|------------|--------| -| 006 | Keep the successful render path synchronous and allocation-light | P1 | M | - | TODO | +| 006 | Keep the successful render path synchronous and allocation-light | P1 | M | - | DONE | | 007 | Compile component parameter schemas and add an empty-schema fast lane | P2 | M | 006 | TODO | | 008 | Enforce one storage concurrency budget during legacy reconciliation | P1 | M | - | TODO |