Skip to content

Commit 927fc04

Browse files
kriszypclaude
andcommitted
fix(openapi): restore empty-resources guard to prevent job-worker early reply
Without the guard a job-type worker (resources.size === 0) responds to RESOURCE_OPENAPI_REQUEST before an app worker, silently returning an empty spec — identical to the original bug (#299). Restores the early-return guard so only a worker that actually owns resources replies. When no worker has resources the caller receives a 503 after the timeout, which is more honest than an empty spec. Also fixes the unit tests to properly exercise the guard and the happy-path: - 'does not respond when this thread has no registered resources' verifies the guard - Send/drop tests populate the resources map with a minimal skippable entry (isError: true) so Resources.set() wraps it correctly for generateJsonApi Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 684aac9 commit 927fc04

2 files changed

Lines changed: 25 additions & 14 deletions

File tree

server/itc/serverHandlers.js

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

174174
const { resources } = require('../../resources/Resources.ts');
175+
// Only respond if this thread has registered resources. Job-type workers with an empty
176+
// resources map must stay silent so that an app worker with real resources replies first.
177+
// If no worker has resources the main thread gets a 503 after the timeout, which is a
178+
// more honest response than silently returning an empty spec.
179+
if (!resources || resources.size === 0) return;
175180
const { generateJsonApi } = require('../../resources/openApi.ts');
176181
const openapi = generateJsonApi(resources, event.message.serverHttpURL);
177182

unitTests/server/itc/serverHandlers.test.js

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,15 @@ describe('Test hdbChildIpcHandler module', () => {
225225

226226
describe('Test resourceOpenApiRequestHandler function', () => {
227227
let resource_openapi_handler;
228+
let resources_instance;
228229

229230
before(() => {
230231
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();
232+
resources_instance = resetResources();
233+
});
234+
235+
afterEach(() => {
236+
resources_instance.clear();
234237
});
235238

236239
// Tests validation: invalid events should be rejected and logged
@@ -261,7 +264,9 @@ describe('Test hdbChildIpcHandler module', () => {
261264
expect(log_error_stub).to.have.been.called;
262265
});
263266

264-
it('sends OpenAPI response directly when originator is reachable', async () => {
267+
// Tests guard: a thread with no registered resources must stay silent so that an app
268+
// worker (which has real resources) responds first and the caller gets the correct spec.
269+
it('does not respond when this thread has no registered resources', async () => {
265270
sandbox.resetHistory();
266271
const sendToThreadStub = sandbox.stub(global.threads, 'sendToThread').returns(true);
267272

@@ -271,36 +276,37 @@ describe('Test hdbChildIpcHandler module', () => {
271276
};
272277
await resource_openapi_handler(test_event);
273278

274-
expect(sendToThreadStub).to.have.been.calledOnce;
275-
expect(sendToThreadStub.firstCall.args[0]).to.equal(5);
276-
const responseMessage = sendToThreadStub.firstCall.args[1];
277-
expect(responseMessage.type).to.equal('resource_openapi_response');
278-
expect(responseMessage.message.requestId).to.equal(99);
279-
expect(responseMessage.message.openapi).to.be.an('object');
279+
expect(sendToThreadStub).to.not.have.been.called;
280280
expect(log_error_stub).to.not.have.been.called;
281281
sendToThreadStub.restore();
282282
});
283283

284-
it('sends OpenAPI response even when resources map is empty (no hang)', async () => {
284+
it('sends OpenAPI response directly when originator is reachable', async () => {
285285
sandbox.resetHistory();
286+
// Resources.set wraps the argument as entry.Resource; passing {isError:true} makes
287+
// generateJsonApi skip the entry so the spec is minimal but the send path is exercised.
288+
resources_instance.set('test', { isError: true });
286289
const sendToThreadStub = sandbox.stub(global.threads, 'sendToThread').returns(true);
287290

288291
const test_event = {
289292
type: 'resource_openapi_request',
290-
message: { originator: 5, requestId: 100, serverHttpURL: 'http://localhost:9925' },
293+
message: { originator: 5, requestId: 99, serverHttpURL: 'http://localhost:9925' },
291294
};
292295
await resource_openapi_handler(test_event);
293296

294-
// Worker always responds — caller gets a spec (possibly empty) rather than a 5s timeout
295297
expect(sendToThreadStub).to.have.been.calledOnce;
296-
expect((responseMessage) => responseMessage.message.openapi).to.be.a('function');
298+
expect(sendToThreadStub.firstCall.args[0]).to.equal(5);
297299
const responseMessage = sendToThreadStub.firstCall.args[1];
300+
expect(responseMessage.type).to.equal('resource_openapi_response');
301+
expect(responseMessage.message.requestId).to.equal(99);
298302
expect(responseMessage.message.openapi).to.be.an('object');
303+
expect(log_error_stub).to.not.have.been.called;
299304
sendToThreadStub.restore();
300305
});
301306

302307
it('drops response silently when originator is unreachable', async () => {
303308
sandbox.resetHistory();
309+
resources_instance.set('test', { isError: true });
304310
const sendToThreadStub = sandbox.stub(global.threads, 'sendToThread').returns(false);
305311

306312
const test_event = {

0 commit comments

Comments
 (0)