Skip to content

fix: surface server error body & cap scenarios #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SavioBS629 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: surface server error body & cap scenarios #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
31edfb5
fix: surface server error body on non-2xx responses
SavioBS629 Jun 19, 2026
5f37f10
fix: batch requests to <=10, harden polling, cap scenarios
SavioBS629 Jun 19, 2026
ea3718a
Merge branch 'main' of github.com:SavioBS629/mcp-server into fix/PMAA…
SavioBS629 Jun 19, 2026
039dbd0
fix: removal of deadcode
SavioBS629 Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/lib/apiClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,18 @@ class ApiClient {
const res = await fn(this.axiosAgent);
return new ApiResponse<T>(res);
} catch (error: any) {
if (error.response && !raise_error) {
return new ApiResponse<T>(error.response);
if (error.response) {
if (!raise_error) {
return new ApiResponse<T>(error.response);
}
const body = error.response.data;
const serverMessage =
typeof body === "string"
? body
: (body?.message ?? body?.error ?? JSON.stringify(body));
if (serverMessage) {
error.message = `Request failed with status code ${error.response.status}: ${serverMessage}`;
}
}
throw error;
}
Expand Down
224 changes: 157 additions & 67 deletions src/tools/testmanagement-utils/TCG-utils/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,27 @@ import {
FETCH_DETAILS_URL,
FORM_FIELDS_URL,
BULK_CREATE_URL,
TC_DETAILS_MAX_BATCH,
BULK_CREATE_MAX_BATCH,
MAX_SCENARIOS_PER_DOCUMENT,
} from "./config.js";
import {
DefaultFieldMaps,
Scenario,
CreateTestCasesFromFileArgs,
} from "./types.js";
import { createTestCasePayload } from "./helpers.js";
import {
createTestCasePayload,
chunkArray,
canAcceptScenario,
} from "./helpers.js";
import { getBrowserStackAuth } from "../../../lib/get-auth.js";
import { BrowserStackConfig } from "../../../lib/types.js";
import { getTMBaseURL } from "../../../lib/tm-base-url.js";
import logger from "../../../logger.js";

const POLL_INTERVAL_MS = 10000;
const MAX_POLL_DURATION_MS = 8 * 60 * 1000;

/**
* Fetch default and custom form fields for a project.
Expand Down Expand Up @@ -132,6 +143,7 @@ export async function fetchTestCaseDetails(
export async function pollTestCaseDetails(
traceRequestId: string,
config: BrowserStackConfig,
deadline: number = Date.now() + MAX_POLL_DURATION_MS,
): Promise<Record<string, any>> {
const detailMap: Record<string, any> = {};
let done = false;
Expand All @@ -140,18 +152,27 @@ export async function pollTestCaseDetails(

while (!done) {
// add a bit of jitter to avoid synchronized polling storms
await new Promise((r) => setTimeout(r, 10000 + Math.random() * 5000));
await new Promise((r) =>
setTimeout(r, POLL_INTERVAL_MS + Math.random() * 5000),
);

// Give up before the backend key TTL expires; return whatever we collected.
if (Date.now() > deadline) break;

const poll = await apiClient.post({
url: `${TCG_POLL_URL_VALUE}?x-bstack-traceRequestId=${encodeURIComponent(traceRequestId)}`,
headers: {
"API-TOKEN": getBrowserStackAuth(config),
},
body: {},
// Don't throw on a non-2xx: an expired request key returns 400
// ("Request ids does not exists") and simply means there is nothing more
// to fetch — stop gracefully instead of failing the whole run.
raise_error: false,
});

if (!poll.data.data.success) {
throw new Error(`Polling failed: ${poll.data.data.message}`);
if (poll.status !== 200 || !poll.data?.data?.success) {
break;
}

for (const msg of poll.data.data.message) {
Expand Down Expand Up @@ -189,29 +210,55 @@ export async function pollScenariosTestDetails(
let iteratorCount = 0;
const tmBaseUrl = await getTMBaseURL(config);
const TCG_POLL_URL_VALUE = TCG_POLL_URL(tmBaseUrl);
const deadline = Date.now() + MAX_POLL_DURATION_MS;

// Promisify interval-style polling using a wrapper
await new Promise<void>((resolve, reject) => {
const intervalId = setInterval(async () => {
let stopped = false;

const pollOnce = async () => {
if (stopped) return;
try {
const poll = await apiClient.post({
url: `${TCG_POLL_URL_VALUE}?x-bstack-traceRequestId=${encodeURIComponent(traceId)}`,
headers: {
"API-TOKEN": getBrowserStackAuth(config),
},
body: {},
raise_error: false,
});

if (poll.status !== 200) {
clearInterval(intervalId);
reject(new Error(`Polling error: ${poll.statusText || poll.status}`));
stopped = true;
if (Object.keys(scenariosMap).length > 0) {
resolve();
} else {
reject(
new Error(
`Polling error: ${poll.status} ${typeof poll.data === "string" ? poll.data : JSON.stringify(poll.data)}`,
),
);
}
return;
}

let terminated = false;
for (const msg of poll.data.data.message) {
if (msg.type === "scenario") {
msg.data.scenarios.forEach((sc: any) => {
scenariosMap[sc.id] = { id: sc.id, name: sc.name, testcases: [] };
if (
canAcceptScenario(
scenariosMap,
sc.id,
MAX_SCENARIOS_PER_DOCUMENT,
)
) {
scenariosMap[sc.id] ||= {
id: sc.id,
name: sc.name,
testcases: [],
};
}
});
const count = Object.keys(scenariosMap).length;
await context.sendNotification({
Expand All @@ -227,23 +274,32 @@ export async function pollScenariosTestDetails(

if (msg.type === "testcase") {
const sc = msg.data.scenario;
if (sc) {
if (
sc &&
canAcceptScenario(scenariosMap, sc.id, MAX_SCENARIOS_PER_DOCUMENT)

@SavioBS629 SavioBS629 Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] Drop site for the silent scenario cap

This canAcceptScenario(..., MAX_SCENARIOS_PER_DOCUMENT) gate (and the matching one in the scenario branch) silently skips every scenario past the 10th — fetchTestCaseDetails is never called for it and its test cases are never created. Combined with the capped total in the result string, the user gets no signal that data was dropped. See the config.ts comment for the full finding and suggested fix.

Suggestion: Remove the per-document cap (chunking already handles the real per-request limit), or surface dropped scenarios in the result string.

Reviewer: stack:code-review

) {
const array = Array.isArray(msg.data.testcases)
? msg.data.testcases
: msg.data.testcases
? [msg.data.testcases]
: [];
const ids = array.map((tc: any) => tc.id || tc.test_case_id);

const reqId = await fetchTestCaseDetails(
documentId,
folderId,
projectReferenceId,
ids,
source,
config,
const ids: string[] = array.map(
(tc: any) => tc.id || tc.test_case_id,
);
detailPromises.push(pollTestCaseDetails(reqId, config));

for (const idChunk of chunkArray(ids, TC_DETAILS_MAX_BATCH)) {
const reqId = await fetchTestCaseDetails(
documentId,
folderId,
projectReferenceId,
idChunk,
source,
config,
);
detailPromises.push(
pollTestCaseDetails(reqId, config, deadline),
);
}

scenariosMap[sc.id] ||= {
id: sc.id,
Expand All @@ -267,20 +323,41 @@ export async function pollScenariosTestDetails(
}

if (msg.type === "termination") {
clearInterval(intervalId);
resolve();
terminated = true;
}
}

if (terminated || Date.now() > deadline) {
stopped = true;
logger.info(
`TCG scenario poll stopped (${terminated ? "termination received" : "max duration reached"}); ${Object.keys(scenariosMap).length} scenarios, ${detailPromises.length} detail fetches`,
);
resolve();
return;
}
setTimeout(pollOnce, POLL_INTERVAL_MS);
} catch (err) {
clearInterval(intervalId);
stopped = true;
reject(err);
}
}, 10000); // 10 second interval
};
setTimeout(pollOnce, POLL_INTERVAL_MS);
});

// once all detail fetches are triggered, wait for them to complete
const detailsList = await Promise.all(detailPromises);
const allDetails = detailsList.reduce((acc, cur) => ({ ...acc, ...cur }), {});
const detailsList = await Promise.allSettled(detailPromises);
const rejectedDetails = detailsList.filter(
(r) => r.status === "rejected",
).length;
if (rejectedDetails > 0) {
logger.info(
`TCG detail fetches: ${detailsList.length - rejectedDetails}/${detailsList.length} succeeded, ${rejectedDetails} failed (degrading gracefully)`,
);
}
const allDetails = detailsList.reduce<Record<string, any>>(
(acc, result) =>
result.status === "fulfilled" ? { ...acc, ...result.value } : acc,
{},
);

// attach the fetched detail objects back to each testcase
for (const scenario of Object.values(scenariosMap)) {
Expand All @@ -307,41 +384,65 @@ export async function bulkCreateTestCases(
documentId: number,
config: BrowserStackConfig,
): Promise<string> {
const results: Record<string, any> = {};
const total = Object.keys(scenariosMap).length;
let doneCount = 0;
let testCaseCount = 0;
const failedScenarios: string[] = [];
const tmBaseUrl = await getTMBaseURL(config);
const BULK_CREATE_URL_VALUE = BULK_CREATE_URL(tmBaseUrl, projectId, folderId);

for (const { id, testcases } of Object.values(scenariosMap)) {
const testCaseLength = testcases.length;
testCaseCount += testCaseLength;
if (testCaseLength === 0) continue;
const payload = {
test_cases: testcases.map((tc) =>
createTestCasePayload(
tc,
id,
folderId,
fieldMaps,
documentId,
booleanFieldId,
traceId,
if (testcases.length === 0) continue;

const batches = chunkArray(testcases, BULK_CREATE_MAX_BATCH);
let createdInScenario = 0;
let scenarioFailed = false;

for (const batch of batches) {
const payload = {
test_cases: batch.map((tc) =>
createTestCasePayload(
tc,
id,
folderId,
fieldMaps,
documentId,
booleanFieldId,
traceId,
),
),
),
};
};

try {
const resp = await apiClient.post({
url: BULK_CREATE_URL_VALUE,
headers: {
"API-TOKEN": getBrowserStackAuth(config),
"Content-Type": "application/json",
},
body: payload,
});
results[id] = resp.data;
try {
await apiClient.post({
url: BULK_CREATE_URL_VALUE,
headers: {
"API-TOKEN": getBrowserStackAuth(config),
"Content-Type": "application/json",
},
body: payload,
});
createdInScenario += batch.length;
} catch (error) {
scenarioFailed = true;
await context.sendNotification({
method: "notifications/progress",
params: {
progressToken: context._meta?.progressToken ?? traceId,
message: `Creation failed for scenario ${id}: ${error instanceof Error ? error.message : "Unknown error"}`,
total,
progress: doneCount,
},
});
}
}

testCaseCount += createdInScenario;
if (scenarioFailed) {
failedScenarios.push(id);
}
if (createdInScenario > 0) {
doneCount++;
await context.sendNotification({
method: "notifications/progress",
params: {
Expand All @@ -351,23 +452,12 @@ export async function bulkCreateTestCases(
progress: doneCount,
},
});
} catch (error) {
//send notification
await context.sendNotification({
method: "notifications/progress",
params: {
progressToken: context._meta?.progressToken ?? traceId,
message: `Creation failed for scenario ${id}: ${error instanceof Error ? error.message : "Unknown error"}`,
total,
progress: doneCount,
},
});
//continue to next scenario
continue;
}
doneCount++;
}
const resultString = `Total of ${testCaseCount} test cases created in ${total} scenarios.`;
let resultString = `Total of ${testCaseCount} test cases created in ${doneCount} of ${total} scenarios.`;
if (failedScenarios.length > 0) {
resultString += ` Failed to create test cases for ${failedScenarios.length} scenario(s): ${failedScenarios.join(", ")}.`;
}
return resultString;
}

Expand Down
7 changes: 7 additions & 0 deletions src/tools/testmanagement-utils/TCG-utils/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
export const TC_DETAILS_MAX_BATCH = 10;

export const BULK_CREATE_MAX_BATCH = 10;

// Cap scenarios per document (mirrors TCG's former maxScenariosPerDocument=10).
export const MAX_SCENARIOS_PER_DOCUMENT = 10;

@SavioBS629 SavioBS629 Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] Scenarios beyond this cap are silently discarded

When the backend returns more than 10 distinct scenarios, canAcceptScenario rejects every new scenario id once the map holds 10 — those scenarios and all their test cases are dropped (scenario branch never creates the entry; testcase branch skips fetchTestCaseDetails). bulkCreateTestCases then reports "…in N of <total>" where <total> is the capped count, so a 14-scenario document reports "10 of 10" — the user never learns 4 were dropped.

The real backend constraint is 10 IDs per fetch-details request, already handled correctly by chunkArray(ids, TC_DETAILS_MAX_BATCH). This separate per-document scenario cap looks reverse-engineered from a constant that doesn't exist on the backend.

Suggestion: Drop MAX_SCENARIOS_PER_DOCUMENT capping entirely. If a safety valve is genuinely wanted, track dropped scenario ids and surface them in the result string, and justify the value against the backend.

Reviewer: stack:code-review

@SavioBS629 SavioBS629 Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] Scenarios beyond this cap are still silently discarded (unresolved)

Both the scenario and testcase branches gate on canAcceptScenario(scenariosMap, sc.id, MAX_SCENARIOS_PER_DOCUMENT). When the backend returns >10 distinct scenarios, the extras and all their test cases are dropped, and bulkCreateTestCases reports "…in <doneCount> of <total> scenarios" where total is the capped Object.keys(scenariosMap).length — so a 14-scenario doc reports "10 of 10" with no truncation notice.

The real backend constraint is 10 IDs per fetch-details request, already enforced by chunkArray(ids, TC_DETAILS_MAX_BATCH). This separate per-document scenario cap is reverse-engineered from a constant that isn't a real hard backend limit, and it's untested (no test covers the drop path).

Suggestion: Remove MAX_SCENARIOS_PER_DOCUMENT + the canAcceptScenario gating entirely. If a per-document cap is genuinely required, surface it — track the true incoming count and append "Note: N scenarios exceeded the per-document limit and were not created" to the result string, plus a test asserting the >10 case.

Reviewer: stack:code-review


export const TCG_TRIGGER_URL = (baseUrl: string) =>
`${baseUrl}/api/v1/integration/tcg/test-generation/suggest-test-cases`;

Expand Down
Loading