Skip to content

rest: classifyDataError's structured-code arms ship the QuickJS debug wrapper as error for a sandboxed producer — the mirror of #11588, on the door #11588 did not touch #14704

Description

@os-trump

Found while implementing #14541 (ordering between resolveErrorResponse's declared-status passthrough and the bespoke structured arms). That card's fix is fenced to the ORDERING, and closing this one means deciding which sentence the arms read — a separate contract question — so it is filed rather than ridden on its PR.

Measured

packages/rest/src/error-response.ts at d2afe5b91d (the #14541 branch; the same measurement holds on origin/main c616c2cc20, where the arms are inline in classifyDataError rather than lifted). One SandboxError-shaped error — message carrying the QuickJS debug wrapper, innerMessage carrying the business sentence, exactly what runtime/src/sandbox/quickjs-runner.ts produces — handed to both REST exits:

error = { message: "hook 'guard' threw: Error: Opportunity is closed.",
          innerMessage: 'Opportunity is closed.',
          code: 'DELETE_RESTRICTED', status: 409,
          object: 'account', dependentObject: 'contact' }

sendThrownError / handleRouteError   (the bulk / metadata / UI door)
  409 {"error":"Opportunity is closed.","code":"DELETE_RESTRICTED"}

mapDataError                          (the single-record /data door)
  409 {"error":"hook 'guard' threw: Error: Opportunity is closed.","code":"DELETE_RESTRICTED",
       "dependentObject":"contact","object":"account"}

The bulk door is correct — that is #11588's sandboxBusinessMessage read on the declared-status passthrough. The single-record door ships the wrapper.

Mechanism

classifyDataError's structured-code arms are surfaced ahead of the sandbox unwrap door, deliberately, "so the structured fields survive the generic catch-alls" — and every one of them builds its sentence from error?.message. For a sandboxed producer error.message IS the debug wrapper, and the unwrap door that would have read innerMessage sits below them and is never reached.

#11588 repaired the same class one door over: it named sandboxBusinessMessage and taught the declared-status passthrough in resolveErrorResponse to read it, because that arm "shipped the QuickJS DEBUG WRAPPER to the end user … One hook, one refusal, two different sentences depending on which route the caller happened to use." The arms in classifyDataError were not in that card's scope. So the wrapper still reaches a client — now on the by-id door rather than the bulk one, i.e. the direction reversed rather than closed.

#14389 already recognised the shape from the other side and worked around it locally: its DUPLICATE_RECORD arm is gated on name === 'DuplicateRecordError' rather than on the code alone, precisely so "a hook that deliberately throws the registered DUPLICATE_RECORD from a sandbox body … keeps the answer the sandbox unwrap door gives it today … rather than having its sentence swapped for this one and the QuickJS debug wrapper shipped as developerMessage." That reasoning applies to every sibling arm; only that one arm carries the guard.

Which arms

Every arm whose gate is a bare code (or a name a sandbox error can carry) and whose body reads error?.message:

DELETE_RESTRICTED · CONCURRENT_UPDATE · ERR_DATASOURCE_UNAVAILABLE · VALIDATION_FAILED · FEEDS_DISABLED / FILES_DISABLED · ATTACHMENT_PARENT_ACCESS / ATTACHMENT_DELETE_DENIED · RECORD_NOT_ACCESSIBLE · PERMISSION_DENIED.

DUPLICATE_RECORD is exempt (the name gate above). OBJECT_NOT_FOUND and INVALID_FIELD are not affected on main — they sit BELOW the unwrap door, so a sandboxed producer reaches the unwrap door first; on the #14541 branch they carry an explicit !isSandboxOrigin clause that preserves exactly that.

VALIDATION_FAILED is the most plausible producer of the three: a metadata-app hook writing throw Object.assign(new Error('Amount must be positive'), { code: 'VALIDATION_FAILED', fields: […] }) is the documented way for an app author to refuse a write with per-field detail, and on the by-id /data door it currently answers 400 {"error":"hook 'guard' threw: Error: Amount must be positive", …}.

What is NOT claimed

Shape of a fix (for triage, not decided here)

The arms need the same two-read rule the unwrap door and the passthrough already share — sandboxBusinessMessage(error) ?? error.message — rather than a third local opinion. The contract question triage would need to settle is what a sandboxed CRASH (isScriptFaultMessage, #7543) should get from an arm gated on a declared code: today the arms answer it with the TypeError prose, where the unwrap door's terminal is the sanitised 500. That is a fault-classification decision, not a message-source one, which is why it is not folded into #14541.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions