mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Ret bc-mcp: config-skabelon matcher broen + fejl svaelges ikke laengere
Live incident i aften: businesscentral MCP fejlede med et generisk 30-sekunders "connection timed out", ingen brugbar fejl. To reelle, adskilte fejl fundet ved at teste direkte mod BC's endpoint: 1. ~/.bc-mcp.config.json havde "company": "CURABIS ApS" (Vist navn), men BC's faktiske Navn-felt er "Curabis ApS". BC svarede korrekt og hurtigt (400, under 200ms) - problemet var aldrig BC. 2. bc-mcp-bridge.js svaelgede det svar stille: en fejl-krop formateret som almindelig JSON, men markeret content-type text/event-stream, blev sendt til parseSSE() som kun leder efter "data:"-linjer - fandt ingen, returnerede en tom liste. Broen skrev derfor INGENTING, hverken stdout eller stderr, og Claude Code ventede blot sin egen 30-sekunders timeout ud. Rettet: - forward() tjekker nu !r.ok FOER content-type-forgrening, ubetinget - en fejlrespons naar aldrig parseSSE, uanset hvad serveren paastaar om sin egen content-type. Testet direkte mod det reproducerede scenarie: fejlen vises nu med det samme (5s test-vindue, ikke 30s timeout), med det fulde BC-fejlsvar synligt i baade stdout (JSON-RPC error) og stderr. - bc-mcp.config.template.json matchede slet ikke broens faktiske felter (tenantId/baseUrl vs. broens tenant/company/configurationName) - enhver ny udvikler der udfyldte skabelonen efter dens egne feltnavne ville faa en config der intet virkede med. Rettet til de rigtige feltnavne, plus en eksplicit advarsel om Navn vs. Vist navn i company-feltet. - Mode A's opsaetningsbesked (Step 3b) opdateret til at naevne alle placeholder-felter, ikke kun secret'en. - To nye BCQuality-videnfiler dokumenterer begge fejl til fremtidig fejlsoegning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
9fc6597030
commit
9e66377fa6
5 changed files with 136 additions and 4 deletions
|
|
@ -0,0 +1,61 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: mcp
|
||||
keywords: [businesscentral, mcp, bc-mcp-bridge, error-handling, sse, timeout, troubleshooting]
|
||||
technologies: [al, mcp]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# bc-mcp-bridge.js Must Check `!r.ok` Before Any SSE Parsing, Unconditionally
|
||||
|
||||
## Description
|
||||
|
||||
`bc-mcp-bridge.js`'s `forward()` function decides how to parse the BC MCP
|
||||
endpoint's response body based on its `content-type` header. Before
|
||||
2026-07-31 it only treated a response as an error when `!r.ok && !text` —
|
||||
i.e. only when there was no body at all. A non-2xx response WITH a body
|
||||
(the common case — BC returns structured JSON error objects) fell through
|
||||
to the content-type branch instead.
|
||||
|
||||
## Incident (2026-07-31)
|
||||
|
||||
BC returned a 400 with a plain JSON error body (`{"Error": {"Message":
|
||||
"..."}}`) while still labelling the response `content-type:
|
||||
text/event-stream`. `parseSSE()` only extracts lines starting with `data:` —
|
||||
a plain JSON body has none, so it returned `[]` silently. The stdin loop's
|
||||
`for (const out of responses) process.stdout.write(...)` then had nothing to
|
||||
iterate, so the bridge produced **zero output** — not even to stderr — for a
|
||||
request that BC had already answered in under 200ms. Claude Code had no
|
||||
signal to work with and waited out its own 30-second client-side timeout,
|
||||
which was the only thing the developer actually saw.
|
||||
|
||||
## Rule
|
||||
|
||||
Check `!r.ok` before considering content-type at all, and throw
|
||||
unconditionally (with the body text included) when it's true. Never let a
|
||||
non-2xx response reach `parseSSE` — a server is free to mislabel an error
|
||||
body's content-type, and the client must not depend on that label being
|
||||
honest.
|
||||
|
||||
## Anti-Pattern
|
||||
|
||||
const ct = r.headers.get("content-type") || "";
|
||||
const text = await r.text();
|
||||
if (!r.ok && !text) throw new Error(`HTTP ${r.status}`);
|
||||
return ct.includes("text/event-stream") ? parseSSE(text) : [text.trim()];
|
||||
// A 400 WITH a body silently falls through to parseSSE and returns [].
|
||||
|
||||
## Compliant
|
||||
|
||||
const ct = r.headers.get("content-type") || "";
|
||||
const text = await r.text();
|
||||
if (!r.ok) throw new Error(`HTTP ${r.status}: ${text || "(empty body)"}`);
|
||||
return ct.includes("text/event-stream") ? parseSSE(text) : [text.trim()];
|
||||
|
||||
## Scope
|
||||
|
||||
`bc-mcp-bridge.js` specifically, but the underlying principle generalizes to
|
||||
any stdio MCP bridge that branches parsing logic on a server-supplied
|
||||
content-type header: validate the HTTP status first, independent of what
|
||||
the header claims the body's shape is.
|
||||
|
|
@ -0,0 +1,57 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: mcp
|
||||
keywords: [businesscentral, mcp, bc-mcp-bridge, company, header, display-name, config, troubleshooting]
|
||||
technologies: [al, mcp]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# BC MCP `Company` Header Must Be the Exact `Navn` Field, Not `Vist navn`
|
||||
|
||||
## Description
|
||||
|
||||
`~/.bc-mcp.config.json`'s `company` value is sent as the literal `Company`
|
||||
HTTP header to the BC MCP endpoint. It must exactly match the company's
|
||||
**`Navn`** field in Business Central's company list — not the **`Vist navn`**
|
||||
(display name) field. The two are often different strings for the same
|
||||
company, and BC's own company picker UI shows the display name more
|
||||
prominently, making it the natural (wrong) one to copy.
|
||||
|
||||
## Incident (2026-07-31)
|
||||
|
||||
CURABIS's own `businesscentral` MCP server failed after "working all
|
||||
evening" with a generic client-side "connection timed out after 30000ms" —
|
||||
no useful error surfaced to the developer. Root cause: `company` was set to
|
||||
`"CURABIS ApS"` (the `Vist navn`), while BC's actual `Navn` field is
|
||||
`"Curabis ApS"`. BC's own API rejected the mismatched header with a fast,
|
||||
clear 400 error — but `bc-mcp-bridge.js` had a separate bug
|
||||
(`bc-mcp-bridge-must-surface-non-2xx-responses-before-sse-parsing`, same
|
||||
incident) that swallowed the error body, turning a sub-200ms server error
|
||||
into a 30-second client-side hang with no diagnostic.
|
||||
|
||||
## Verification
|
||||
|
||||
If `businesscentral` MCP fails, check BC's company list page (Virksomheder /
|
||||
Companies) and compare the `Navn` column — not `Vist navn` — against
|
||||
`~/.bc-mcp.config.json`'s `company` value, character for character. Do not
|
||||
assume the value that "looks right" from the picker UI is the one the API
|
||||
needs.
|
||||
|
||||
## Anti-Pattern
|
||||
|
||||
// WRONG: copied from BC's company switcher, which shows Vist navn
|
||||
{ "company": "CURABIS ApS" }
|
||||
|
||||
## Compliant
|
||||
|
||||
// CORRECT: copied from the Navn column on the company list page
|
||||
{ "company": "Curabis ApS" }
|
||||
|
||||
## Scope
|
||||
|
||||
Every machine with `~/.bc-mcp.config.json` configured — this is a
|
||||
machine-local file, not something Mode B can fix centrally. The template
|
||||
(`bc-mcp.config.template.json`) carries an explicit warning about this
|
||||
distinction as of 2026-07-31, but a machine already onboarded before that
|
||||
date needs its existing file checked manually.
|
||||
|
|
@ -92,7 +92,14 @@ async function forward(msg) {
|
|||
const sid = r.headers.get("mcp-session-id"); if (sid) sessionId = sid;
|
||||
const ct = r.headers.get("content-type") || "";
|
||||
const text = await r.text();
|
||||
if (!r.ok && !text) throw new Error(`HTTP ${r.status}`);
|
||||
// 2026-07-31: BC has returned error bodies as plain JSON while still labelling
|
||||
// content-type text/event-stream (e.g. "company not found"). parseSSE only
|
||||
// extracts lines starting with "data:" - a plain JSON error body has none, so
|
||||
// it silently returned []. The stdin loop then wrote nothing at all, and the
|
||||
// client (Claude Code) waited out its own 30s timeout instead of seeing the
|
||||
// real error immediately. Check !r.ok BEFORE any SSE parsing, unconditionally -
|
||||
// never let a non-2xx response fall through to parseSSE.
|
||||
if (!r.ok) throw new Error(`HTTP ${r.status}: ${text || "(empty body)"}`);
|
||||
return ct.includes("text/event-stream") ? parseSSE(text) : (text.trim() ? [text.trim()] : []);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -205,7 +205,12 @@ If it does NOT exist:
|
|||
2. Write it to `~/.bc-mcp.config.json` as-is
|
||||
3. Tell the developer:
|
||||
> "⚠️ `~/.bc-mcp.config.json` er oprettet fra CURABIS-template.
|
||||
> Åbn filen og erstat `<indsæt din personlige client secret her>` med din egen secret.
|
||||
> Udfyld ALLE placeholder-felter (tenant, clientId, client secret, company)
|
||||
> — ikke kun secret'en. For `company`: brug PRÆCIS firmanavnet fra BC's
|
||||
> 'Navn'-kolonne på virksomhedslisten, IKKE 'Vist navn' — de to kan være
|
||||
> forskellige strenge for samme firma (2026-07-31: 'CURABIS ApS' vs.
|
||||
> 'Curabis ApS' forårsagede et 30-sekunders timeout uden brugbar fejl —
|
||||
> se `bc-mcp-company-header-must-match-exact-company-name`).
|
||||
> Gem filen — BC MCP er klar når du genstarter Claude Code."
|
||||
|
||||
#### 3c. bcquality-knowledge, roster agents, find-altool.ps1, MCP registration (v24)
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
{
|
||||
"tenantId": "CURABIS-TENANT-ID",
|
||||
"tenant": "CURABIS-TENANT-ID",
|
||||
"clientId": "CURABIS-CLIENT-ID",
|
||||
"clientSecret": "<indsæt din personlige client secret her>",
|
||||
"baseUrl": "https://api.businesscentral.dynamics.com"
|
||||
"environment": "Production",
|
||||
"company": "<PRÆCIS firmanavnet fra BC's 'Navn'-felt - IKKE 'Vist navn'. 2026-07-31: 'CURABIS ApS' (Vist navn) fejlede med 'company not found' hos BC MCP; det korrekte var 'Curabis ApS' (Navn). Tjek Virksomheder-siden i BC, kolonnen 'Navn', hvis usikker.>",
|
||||
"configurationName": "CURABIS_DEV"
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue