Skip to content

Commit 8511c30

Browse files
committed
refactor(theia): use invalid-response reason for non-record bridge replies
Codex follow-up nit on the previous commit: the union member 'invalid-response' was reachable in theiaEnvironment's switch but the reader never actually returned it (string responses fell through to 'timeout' after the polling loop). The arktype validation string the bridge emits when called with the wrong shape will not change between polls, so polling for 10s is wasted time. Now: non-record / non-array responses (string, array, primitive) cause an immediate fail-fast return of 'invalid-response' with the actual response captured for diagnostics. Genuine timeouts (empty record or thrown errors throughout) still return 'timeout'.
1 parent a389e2f commit 8511c30

2 files changed

Lines changed: 33 additions & 19 deletions

File tree

extension/src/extension/theia/dataBridgeReader.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export async function readEnvVarsViaDataBridge<T extends string>(
8484

8585
logger.info('DataBridge enabled, polling for environment variables...', LogCategory.GENERAL);
8686
const deadline = Date.now() + POLL_TIMEOUT_MS;
87-
let lastInvalidResponse: string | undefined;
87+
let lastError: string | undefined;
8888

8989
while (Date.now() < deadline) {
9090
try {
@@ -97,7 +97,7 @@ export async function readEnvVarsViaDataBridge<T extends string>(
9797
[...names],
9898
);
9999

100-
if (envMap && typeof envMap === 'object') {
100+
if (envMap && typeof envMap === 'object' && !Array.isArray(envMap)) {
101101
if (names.every((name) => !!(envMap as Record<string, string>)[name])) {
102102
logger.info('DataBridge: all environment variables received', LogCategory.GENERAL);
103103
const result = {} as Record<T, string | undefined>;
@@ -106,15 +106,23 @@ export async function readEnvVarsViaDataBridge<T extends string>(
106106
}
107107
return { kind: 'success', env: result };
108108
}
109-
} else if (typeof envMap === 'string') {
110-
// Bridge returned its error summary string — keep the latest
111-
// for diagnostics if the deadline runs out before recovery.
112-
lastInvalidResponse = envMap;
109+
// Record present but missing keys — keep polling; values may
110+
// arrive in a later POST.
111+
} else if (envMap !== undefined && envMap !== null) {
112+
// Non-record response (string, array, primitive). The bridge is
113+
// responding but rejecting the call, so polling will not help —
114+
// the request shape is wrong and won't change. Fail fast with
115+
// the actual response captured for diagnostics.
116+
return {
117+
kind: 'failure',
118+
reason: 'invalid-response',
119+
details: typeof envMap === 'string' ? envMap : `unexpected response type: ${typeof envMap}`,
120+
};
113121
}
114122
} catch (e) {
115123
// data-bridge may not be ready yet — keep polling. Capture the
116124
// most recent error to surface if the deadline elapses.
117-
lastInvalidResponse = e instanceof Error ? e.message : String(e);
125+
lastError = e instanceof Error ? e.message : String(e);
118126
}
119127

120128
await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS));
@@ -124,7 +132,7 @@ export async function readEnvVarsViaDataBridge<T extends string>(
124132
'DataBridge: timeout waiting for environment variables',
125133
LogCategory.GENERAL,
126134
);
127-
return { kind: 'failure', reason: 'timeout', details: lastInvalidResponse };
135+
return { kind: 'failure', reason: 'timeout', details: lastError };
128136
}
129137

130138
interface DataBridgeProbeResult {

extension/test/unit/theia/dataBridgeReader.test.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,30 +90,36 @@ suite('readEnvVarsViaDataBridge', () => {
9090
assert.ok(setTimeoutStub.callCount >= 1);
9191
});
9292

93-
test('returns failure with reason=timeout and details when bridge returns its error string', async () => {
93+
test('returns failure with reason=invalid-response when bridge returns a string error', async () => {
9494
process.env.DATA_BRIDGE_ENABLED = '1';
9595
sandbox.stub(vscode.commands, 'getCommands').resolves(['dataBridge.getEnv']);
9696
sandbox.stub(vscode.commands, 'executeCommand').resolves('arktype validation error: expected string[]');
9797

98-
const startTime = 1_000_000_000;
99-
let nowOffset = 0;
100-
sandbox.stub(Date, 'now').callsFake(() => startTime + nowOffset);
101-
sandbox.stub(global, 'setTimeout').callsFake((fn: TimerHandler) => {
102-
nowOffset += 11_000;
103-
if (typeof fn === 'function') { (fn as () => void)(); }
104-
return 0 as unknown as NodeJS.Timeout;
105-
});
106-
98+
// No setTimeout/Date stubs needed — the function should fail fast on
99+
// the first non-record response without polling further.
107100
const result = await readEnvVarsViaDataBridge(KEYS);
108101

109102
assert.strictEqual(result.kind, 'failure');
110103
if (result.kind === 'failure') {
111-
assert.strictEqual(result.reason, 'timeout');
104+
assert.strictEqual(result.reason, 'invalid-response');
112105
assert.ok(result.details && result.details.includes('arktype'),
113106
`expected details to include the bridge error string, got: ${result.details}`);
114107
}
115108
});
116109

110+
test('returns failure with reason=invalid-response when bridge returns an array', async () => {
111+
process.env.DATA_BRIDGE_ENABLED = '1';
112+
sandbox.stub(vscode.commands, 'getCommands').resolves(['dataBridge.getEnv']);
113+
sandbox.stub(vscode.commands, 'executeCommand').resolves(['unexpected'] as unknown as Record<string, string>);
114+
115+
const result = await readEnvVarsViaDataBridge(KEYS);
116+
117+
assert.strictEqual(result.kind, 'failure');
118+
if (result.kind === 'failure') {
119+
assert.strictEqual(result.reason, 'invalid-response');
120+
}
121+
});
122+
117123
test('returns success with all requested keys when bridge delivers them', async () => {
118124
process.env.DATA_BRIDGE_ENABLED = '1';
119125
sandbox.stub(vscode.commands, 'getCommands').resolves(['dataBridge.getEnv']);

0 commit comments

Comments
 (0)