fix: preserve original error info on unexpected deploy failures

[LautaroPetaccio]

Why

When a deployment hits the last-resort catch in deployEntity (content/src/logic/deployment-service/component.ts), the original exception was discarded and replaced with a constant string "There was an error deploying the entity" before returning an InvalidResult. That string is the only thing that propagates through deployDownloadedEntity (which wraps it as Errors deploying entity(<id>):\n - <message> and throws) into batch-deployer, where it gets persisted as failed_deployments.error_description via err.toString().

The result: every unexpected deploy failure looked identical in failed_deployments and in the snapshot output, regardless of whether the cause was a database error, a storage write failure, or an unhandled throw out of pointerManager / activeEntities. The full error was logged to the server log on line 417, but once that log rotates the failure record is the only thing left — and it carries no diagnostic value. Investigations of old failures were effectively unrecoverable.

This catch only fires for genuinely unexpected throws. Known validator failures still flow through validateDeployment's {ok: false, errors} return path (handled at line 360), so user-facing validation messages ("does not have access to parcel ...", schema errors, etc.) are unaffected.

How

Append ${error.name}: ${error.message} to the persisted message:

} catch (error: any) {
  logger.error(`There was an error deploying the entity: ${error}`, { entityId })
  const name = error?.name ?? 'Error'
  const message = error?.message ?? 'unknown error'
  return InvalidResult({
    errors: [`There was an error deploying the entity: ${name}: ${message}`]
  })
}

• The full ${error} interpolation in the server log is unchanged.
• Optional chaining + fallbacks handle the case where something non-Error was thrown (string, undefined, plain object).
• No tests assert on the old exact string (verified with grep -rln "There was an error deploying the entity" src test).

Trade-off worth flagging in review

The new message surfaces error.message to failed_deployments.error_description, which is exposed via the failed-deployments endpoint and propagates to other catalysts in snapshots. The throwers reachable from this catch (storeEntityContent, the tx_deploy_entity transaction body, pointerManager.referenceEntityFromPointers, activeEntities updates) don't currently embed sensitive details (connection strings, internal hostnames, file paths) in their messages, so this is fine today — but if a future thrower does, that detail would leak. If the team prefers a conservative posture, an easy follow-up is to persist only error.name (or a category derived from it) and keep the full message log-only.

Test plan

• Trigger a synthetic throw inside the wrapped block (e.g. force storeEntityContent to reject) and confirm failed_deployments.error_description contains the error name and message.
• Confirm a normal validator-rejected deployment still surfaces the validator's specific errors (this path is untouched but worth a sanity check).
• CI build / lint passes.

[decentraland-bot]

Review Summary

PR: #1925 — fix: preserve original error info on unexpected deploy failures
Branch: fix/preserve-error-info-on-deploy-failure → main
Files changed: 1 (+4 −2)
CI: ✅ All 5 checks passing

Clean, minimal change with an excellent PR description. The intent is correct — preserving error.name and error.message in the persisted failed_deployments.error_description is a significant diagnostic improvement over the old generic string. The defensive coding (optional chaining + fallbacks) handles non-Error throws correctly.

Findings

#	Sev	Category	Finding
1	P2	Security	Information disclosure risk via unauthenticated GET /failed-deployments
2	P2	TypeScript	catch (error: any) should prefer unknown with type narrowing
3	P2	Testing	No regression test for the new error format

Details

[P2] Information disclosure — forward-looking risk
The error message now flows to an unauthenticated public endpoint (GET /failed-deployments) and is broadcast to all catalysts via snapshot sync. As the PR description correctly identifies, the current throwers (storeEntityContent, tx_deploy_entity, pointerManager, activeEntities) don't embed sensitive details. However, a future thrower from a DB driver or cloud SDK could surface internal hostnames, file paths, or connection metadata.

The PR body already proposes a follow-up: persist only error.name (or a category) and keep the full message log-only. That's a solid approach — consider filing an issue to track it.

[P2] catch (error: any) → prefer catch (error: unknown)
With useUnknownInCatchVariables (strict mode), the idiomatic pattern is:

} catch (error: unknown) {
  const name = error instanceof Error ? error.name : 'Error'
  const message = error instanceof Error ? error.message : String(error)

This preserves type safety for the rest of the catch scope. The current any + optional chaining is functionally equivalent at runtime, but unknown makes the narrowing explicit.

[P2] No test added
The catch block is a last-resort handler that's hard to exercise accidentally. A targeted test (e.g., force storeEntityContent to reject and assert on the InvalidResult error string) would guard against future regressions.

Git conventions (ADR-6)

• PR title: fix: preserve original error info on unexpected deploy failures ✅
• Branch: fix/preserve-error-info-on-deploy-failure ✅

Security checklist

• No secrets or credentials introduced ✅
• No auth/authz changes ✅
• No dependency changes ✅
• Information exposure: P2 (acknowledged by author, not currently exploitable)

Verdict: APPROVE ✅

No P0 or P1 findings. All three P2s are non-blocking improvements. The change is correct, focused, and the diagnostic value is a clear win.

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio via Slack