# Premium Error Boundary Follow-ups

## Analysis

### Impact Table

| Area | File | Change | Verification | Success Criteria |
|------|------|--------|--------------|------------------|
| Premium import subtask failure messaging | `internal/model/import/import_into.go` | Preserve safe agent/ExecSQL payload errors when import execution explicitly marks a subtask failed; `SHOW IMPORT JOB` polling remains on the generic/internal path in this PR | `go test ./internal/model/import -run '^(TestImportIntoSubtaskMessageFromErr|TestMarkImportIntoSubtaskFailedFromErr|TestMarkImportIntoSubtaskFailedFromErrKeepsSanitizedWrappedMessage)$'` | Explicit import-start failures retain sanitized payload messages without claiming new polling behavior |
| Premium DM precheck execution | `internal/model/dm/premium_dm/precheck_item_executor.go` | Keep agent request failures generic and let the API boundary mask internal markers | `go test ./internal/model/dm/premium_dm ./internal/openapi/service/premium_dm -run '^TestPremiumDMPrecheckToV1beta2PbPrecheck$|^TestPremiumDMPrecheckToV1beta2PbPrecheckFallsBackToGenericReason$|^TestPremiumDMPrecheckToV1beta2PbPrecheckHidesInternalFailureReason$|^TestPremiumDMPrecheckToV1beta2PbPrecheckHidesInternalConfigFailureReason$'` | Failed precheck items do not expose transport, gateway, or agent implementation details |
| Premium DM precheck API output | `internal/openapi/service/premium_dm/premium_dm_vo.go` | Restrict user-visible `Reason` to checker `ShortErr` aggregation; failed-stage items always use a generic internal message | Same as above | API `Reason` only surfaces checker result reasons, not failed-item `ErrMsg` |
| Agent ExecSQL payload exposure | `dataflow-agent-ng/agent/grpc/services.go` | Only surface direct MySQL/TiDB execution/query errors through `resp.ErrorMsg`; return driver/transport/setup/internal failures as hard errors | `go test ./dataflow-agent-ng/agent/grpc -run '^TestSQLPayloadErrorMessage$'` | User-facing callers only receive payload errors for actual DB SQL failures |
| Premium import check connection | `internal/openapi/service/importadmin/import_into.go` | Preserve payload DB/business errors in `checkDatabaseConnectivity` while keeping transport/internal failures generic | `go test ./internal/openapi/service/importadmin -run '^(TestFromSourceURI_Azure_StripsSecrets|TestNormalizeAzureBlobInput|TestJoinSourceURI_PreservesQuery|TestPayloadErrorOrFallback|TestCheckDatabaseConnectivityFailureResp)$'` | Endpoint returns actionable connection errors when present and generic fallback otherwise |
| Premium import test bucket | `internal/openapi/service/importadmin/import_into.go` | Return payload S3 bucket/storage errors instead of replacing them with a fixed message | Same as above | S3 test-bucket path matches OSS/Azure business-error behavior |

### Symbols To Touch

- `importIntoSubtaskMessageFromErr`
- `markImportIntoSubtaskFailedFromErr`
- `(*PremiumDMPrecheckWorker).doPremiumPrecheckDMJob`
- `premiumPrecheckFailureReason`
- `sqlPayloadErrorMessage`
- `(*AgentServer).ExecSQLOnCluster`
- `(*ImportIntoService).CheckDatabaseConnectivity`
- `(*ImportIntoService).CheckImportSourceConnection`

### Scope Notes

- This PR keeps the user-facing boundary consistent for premium DM/import flows: payload business/SQL/storage errors may surface; transport, panic, config, and agent implementation failures stay generic/internal.
- Agent `ExecSQL` payloads are further narrowed so only direct MySQL/TiDB server errors are copied into `resp.ErrorMsg`; transport/driver/setup failures must bubble out as hard errors.
- Premium DM precheck request failures stay on the generic internal path; the executor no longer preserves `agent.ErrorResponse.Message` as persisted item text.
- Premium DM precheck `Reason` should only come from checker `ShortErr` aggregation; failed-stage item `ErrMsg` is no longer treated as user-visible reason text.
- Low-level agent row-metadata, scan, and row-iteration failures (`Rows.Columns`, `rows.Scan`, `rows.Err`) stay on the internal error path instead of being surfaced through `resp.ErrorMsg`.
- `premium_dm` does not have a separate bucket-test endpoint; the premium-facing import test-bucket and test-connection flows are the `adminapi v1beta2` import routes in `internal/openapi/service/importadmin/import_into.go`.
- Connection-validation status-code semantics in `internal/openapi/service/premium_dm/premium_dm.go` remain out of scope.
- `CheckImportSource` still has older mixed error behavior and is intentionally not normalized in this PR.

### Follow-up Outcome

- Premium DM precheck reverted the temporary `agent.ErrorResponse` passthrough and now persists the generic `failed to precheck dm job` message for request failures.
- The removed helper/test coverage was folded into the final premium precheck verification set below; there is no separate follow-up document for this revert.

## Execution Steps

1. Update premium import subtask failure messaging in `internal/model/import/import_into.go`.
   - Reuse `importIntoSubtaskMessageFromErr` when import execution explicitly marks a subtask failed; do not claim any `SHOW IMPORT JOB` polling change in this PR.
   - Verification: `go test ./internal/model/import -run '^(TestImportIntoSubtaskMessageFromErr|TestMarkImportIntoSubtaskFailedFromErr|TestMarkImportIntoSubtaskFailedFromErrKeepsSanitizedWrappedMessage)$'`.

2. Update premium DM precheck executor and API conversion.
   - Keep request failures on the generic internal path and stop using failed-item `ErrMsg` as a user-visible reason source.
   - Verification: `go test ./internal/model/dm/premium_dm ./internal/openapi/service/premium_dm -run '^TestPremiumDMPrecheckToV1beta2PbPrecheck$|^TestPremiumDMPrecheckToV1beta2PbPrecheckFallsBackToGenericReason$|^TestPremiumDMPrecheckToV1beta2PbPrecheckHidesInternalFailureReason$|^TestPremiumDMPrecheckToV1beta2PbPrecheckHidesInternalConfigFailureReason$'`.

3. Update premium-facing import check endpoints.
   - Keep payload `ErrorMsg` for `checkDatabaseConnectivity` and S3 `checkImportSourceConnection`, while preserving generic fallback for transport/internal failures.
   - Verification: `go test ./internal/openapi/service/importadmin -run '^(TestFromSourceURI_Azure_StripsSecrets|TestNormalizeAzureBlobInput|TestJoinSourceURI_PreservesQuery|TestPayloadErrorOrFallback|TestCheckDatabaseConnectivityFailureResp)$'`.

4. Tighten the NG agent `ExecSQL` payload boundary.
   - Only direct MySQL/TiDB execution/query errors should be returned in `resp.ErrorMsg`; non-SQL failures should return hard errors.
   - Verification: `go test ./dataflow-agent-ng/agent/grpc -run '^TestSQLPayloadErrorMessage$'`.

5. Run final validation for all in-scope follow-ups.
   - `go test ./internal/model/import -run '^(TestImportIntoSubtaskMessageFromErr|TestMarkImportIntoSubtaskFailedFromErr|TestMarkImportIntoSubtaskFailedFromErrKeepsSanitizedWrappedMessage)$'`
   - `go test ./internal/model/dm/premium_dm ./internal/openapi/service/premium_dm -run '^TestPremiumDMPrecheckToV1beta2PbPrecheck$|^TestPremiumDMPrecheckToV1beta2PbPrecheckFallsBackToGenericReason$|^TestPremiumDMPrecheckToV1beta2PbPrecheckHidesInternalFailureReason$|^TestPremiumDMPrecheckToV1beta2PbPrecheckHidesInternalConfigFailureReason$'`
   - `go test ./dataflow-agent-ng/agent/grpc -run '^TestSQLPayloadErrorMessage$'`
   - `go test ./internal/openapi/service/importadmin -run '^(TestFromSourceURI_Azure_StripsSecrets|TestNormalizeAzureBlobInput|TestJoinSourceURI_PreservesQuery|TestPayloadErrorOrFallback|TestCheckDatabaseConnectivityFailureResp)$'`

## Risks

- Premium import check endpoints still leave early request/setup failures (`tidb_id` parse, agent client creation) unchanged in this PR.
- Import progress polling via `syncImportIntoProgress` still returns a generic/internal path on `SHOW IMPORT JOB` execution failures; this document does not claim otherwise.
- Failed-stage precheck items no longer expose persisted `ErrMsg`; any user-visible reason must come from checker result `ShortErr` values.