Skip to content

Commit 634bd9e

Browse files
stephentoubCopilot
andcommitted
Address review feedback: explicit catch and corrected comment
- TestHelper.cs: replace bare `catch { }` in the backfill-drain `finally` with `catch (Exception) { /* intentionally ignored: already propagated via tcs */ }` to make the swallow intent explicit and satisfy CodeQL. - sdkTestHelper.ts: update the comment above `getFutureFinalResponse` to reflect that the live subscription is installed before the existing- messages RPC fires, matching the actual call ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b4b23da commit 634bd9e

2 files changed

Lines changed: 4 additions & 3 deletions

File tree

dotnet/test/Harness/TestHelper.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ void TryComplete()
7878
{
7979
// Drain the backfill before our `using` scopes (cts, subscription) dispose.
8080
// Any exception was already routed through tcs above, so swallow here.
81-
try { await backfill.ConfigureAwait(false); } catch { }
81+
try { await backfill.ConfigureAwait(false); }
82+
catch (Exception) { /* intentionally ignored: already propagated via tcs */ }
8283
}
8384

8485
async Task CheckExistingMessagesAsync()

nodejs/test/e2e/harness/sdkTestHelper.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ export async function getFinalAssistantMessage(
88
session: CopilotSession,
99
{ alreadyIdle = false }: { alreadyIdle?: boolean } = {}
1010
): Promise<AssistantMessageEvent> {
11-
// Start listening for the answer immediately so we don't miss any events that arrive
12-
// between the existing-messages RPC starting and the subscription being installed.
11+
// Install the live subscription (via getFutureFinalResponse) before issuing the
12+
// existing-messages RPC so we don't miss events that arrive while that RPC is in flight.
1313
const futurePromise = getFutureFinalResponse(session);
1414
// We may end up returning from the existing-messages path; attach a noop handler so
1515
// the unawaited future-response rejection doesn't surface as an unhandled rejection.

0 commit comments

Comments
 (0)