Skip to content

Commit 1c3a0b5

Browse files
kriszypclaude
andcommitted
fix(openapi): remove early-return guard so workers always respond to OpenAPI requests
Without this change, a worker with 0 registered resources would silently drop the ITC request, causing the main thread to wait 5 seconds before timing out. Now every worker responds (even with an empty spec), matching Dawson's review request. Also adds a full test suite for resourceOpenApiRequestHandler including happy-path send/drop assertions and a 'no hang on empty resources' case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a6661ca commit 1c3a0b5

2 files changed

Lines changed: 23 additions & 22 deletions

File tree

server/itc/serverHandlers.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,6 @@ async function resourceOpenApiRequestHandler(event) {
172172
hdbLogger.trace(`ITC resourceOpenApiRequestHandler received request:`, event);
173173

174174
const { resources } = require('../../resources/Resources.ts');
175-
if (!resources || resources.size === 0) {
176-
// This thread has no registered resources — don't respond so another worker can.
177-
return;
178-
}
179175
const { generateJsonApi } = require('../../resources/openApi.ts');
180176
const openapi = generateJsonApi(resources, event.message.serverHttpURL);
181177

unitTests/server/itc/serverHandlers.test.js

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const harperBridge = require('#src/dataLayer/harperBridge/harperBridge').default
1212
// Note: rewire is used to access private functions (schemaHandler, userHandler, componentStatusRequestHandler)
1313
// for testing validation logic, not for replacing dependencies with mocks
1414
const server_itc_handlers = rewire('#js/server/itc/serverHandlers');
15+
const { resetResources } = require('#src/resources/Resources');
1516

1617
describe('Test hdbChildIpcHandler module', () => {
1718
const TEST_ERR = 'The roof is on fire';
@@ -227,6 +228,9 @@ describe('Test hdbChildIpcHandler module', () => {
227228

228229
before(() => {
229230
resource_openapi_handler = server_itc_handlers.__get__('resourceOpenApiRequestHandler');
231+
// Ensure the module-level `resources` export is an initialised Resources instance
232+
// (not undefined) so generateJsonApi can iterate over it safely.
233+
resetResources();
230234
});
231235

232236
// Tests validation: invalid events should be rejected and logged
@@ -260,14 +264,6 @@ describe('Test hdbChildIpcHandler module', () => {
260264
it('sends OpenAPI response directly when originator is reachable', async () => {
261265
sandbox.resetHistory();
262266
const sendToThreadStub = sandbox.stub(global.threads, 'sendToThread').returns(true);
263-
// Inject a minimal mock for generateJsonApi and a non-empty resources Map
264-
const mockOpenapi = { openapi: '3.0.3', paths: {} };
265-
const mockResources = new Map([['test', { path: 'test', Resource: { isError: false } }]]);
266-
server_itc_handlers.__set__('require', (path) => {
267-
if (path.includes('Resources')) return { resources: mockResources };
268-
if (path.includes('openApi')) return { generateJsonApi: () => mockOpenapi };
269-
return require(path);
270-
});
271267

272268
const test_event = {
273269
type: 'resource_openapi_request',
@@ -280,22 +276,32 @@ describe('Test hdbChildIpcHandler module', () => {
280276
const responseMessage = sendToThreadStub.firstCall.args[1];
281277
expect(responseMessage.type).to.equal('resource_openapi_response');
282278
expect(responseMessage.message.requestId).to.equal(99);
283-
expect(responseMessage.message.openapi).to.deep.equal(mockOpenapi);
279+
expect(responseMessage.message.openapi).to.be.an('object');
284280
expect(log_error_stub).to.not.have.been.called;
285281
sendToThreadStub.restore();
286-
// Restore original require
287-
server_itc_handlers.__set__('require', require);
282+
});
283+
284+
it('sends OpenAPI response even when resources map is empty (no hang)', async () => {
285+
sandbox.resetHistory();
286+
const sendToThreadStub = sandbox.stub(global.threads, 'sendToThread').returns(true);
287+
288+
const test_event = {
289+
type: 'resource_openapi_request',
290+
message: { originator: 5, requestId: 100, serverHttpURL: 'http://localhost:9925' },
291+
};
292+
await resource_openapi_handler(test_event);
293+
294+
// Worker always responds — caller gets a spec (possibly empty) rather than a 5s timeout
295+
expect(sendToThreadStub).to.have.been.calledOnce;
296+
expect((responseMessage) => responseMessage.message.openapi).to.be.a('function');
297+
const responseMessage = sendToThreadStub.firstCall.args[1];
298+
expect(responseMessage.message.openapi).to.be.an('object');
299+
sendToThreadStub.restore();
288300
});
289301

290302
it('drops response silently when originator is unreachable', async () => {
291303
sandbox.resetHistory();
292304
const sendToThreadStub = sandbox.stub(global.threads, 'sendToThread').returns(false);
293-
const mockResources = new Map([['test', { path: 'test', Resource: { isError: false } }]]);
294-
server_itc_handlers.__set__('require', (path) => {
295-
if (path.includes('Resources')) return { resources: mockResources };
296-
if (path.includes('openApi')) return { generateJsonApi: () => ({}) };
297-
return require(path);
298-
});
299305

300306
const test_event = {
301307
type: 'resource_openapi_request',
@@ -308,7 +314,6 @@ describe('Test hdbChildIpcHandler module', () => {
308314
const traceCalls = log_trace_stub.getCalls().map((call) => String(call.args[0]));
309315
expect(traceCalls.some((msg) => msg.includes('Dropping resource OpenAPI response'))).to.be.true;
310316
sendToThreadStub.restore();
311-
server_itc_handlers.__set__('require', require);
312317
});
313318
});
314319
});

0 commit comments

Comments
 (0)