Skip to content

feat: Error sanitization for user-facing services #413

Open
gokutheengineer wants to merge 11 commits intobsv-blockchain:mainfrom
gokutheengineer:gokhan/concat-error-msgs
Open

feat: Error sanitization for user-facing services #413
gokutheengineer wants to merge 11 commits intobsv-blockchain:mainfrom
gokutheengineer:gokhan/concat-error-msgs

Conversation

@gokutheengineer
Copy link
Collaborator

@gokutheengineer gokutheengineer commented Jan 19, 2026

Error Sanitization for User-Facing Services

Context

We observed verbose error chains leaking internal details (e.g., Aerospike, underlying storage errors, and deep wrapped chains) into user-facing surfaces (propagation, asset server, p2p). This is useful for debugging but not appropriate for external clients.

Goal

Return succinct, safe error messages to users while preserving full error detail in logs and internal chains.

Summary of Changes

  1. Centralized sanitization helpers

    • Added helpers in errors to generate concise, public-safe messages and sanitized errors for gRPC/HTTP.
  2. Propagation service sanitization

    • HTTP endpoints now return short messages (no wrapped chain or data).
    • gRPC errors are wrapped with sanitized status (code + message only).
    • Batch responses return sanitized TError per item.
  3. Asset server sanitization

    • HTTP error responses now use sanitized messages.
  4. P2P API sanitization

    • User-facing gRPC endpoints return sanitized errors instead of raw internal chains.

Detailed Changes

New error helpers (errors/errors.go)

  • UserMessage(err)
    Returns a concise message like TX_INVALID (31): tx invalid (no wrapped chain, no data).
  • PublicError(err)
    Creates a sanitized *errors.Error with only code + message.
  • WrapGRPCPublic(err)
    Produces a gRPC status with only the sanitized code/message.
  • WrapPublic(err)
    Converts to sanitized *errors.TError (used in batch responses).

Propagation (services/propagation/Server.go)

  • /tx and /txs HTTP error responses now use errors.UserMessage(err)
    (removes internal storage/db details and long chains).
  • gRPC ProcessTransaction returns WrapGRPCPublic(err) to keep responses minimal.
  • Batch ProcessTransactionBatch uses WrapPublic(err) per item to strip details.

Asset server (services/asset/httpimpl/sendError.go)

  • JSON error response now uses errors.UserMessage(err) instead of err.Error().

P2P (services/p2p/Server.go)

  • Sanitized errors for user-facing endpoints:
    • GetPeers when P2P client not initialized
    • BanPeer, UnbanPeer errors
    • RecordBytesDownloaded decode errors

Example Before/After

Before (too verbose):

PROCESSING (4): [Validate][txid] error spending utxos -> PROCESSING (4): validator: UTXO Store spend failed ... -> TX_INVALID (31): error in aerospike spend ... -> 70: UTXO_SPENT (70): ... "utxo ... already spent by ..."

After (sanitized):

TX_INVALID (31): tx invalid

or

UTXO_SPENT (70): utxo already spent

Files Touched

  • errors/errors.go
  • services/propagation/Server.go
  • services/asset/httpimpl/sendError.go
  • services/p2p/Server.go

Risks / Considerations

  • Less detail for external users; internal logs still preserve full chains.
  • gRPC clients now receive sanitized errors, so any client-side logic depending on deep error chains should be reviewed.

Follow-ups (Optional)

  • Extend sanitization to other external surfaces (RPC/validator/legacy APIs).
  • Add a config flag to toggle verbose vs public errors for debug environments.
  • Add tests verifying sanitized responses.

Note

There is an additional change in services/propagation/Server.go to ignore ErrBlobAlreadyExists when storing transactions. This appears separate from sanitization and is not part of the error-sanitization scope.

@gokutheengineer gokutheengineer changed the title feat: concat error messages for user facing errors feat: clenaup error messages for user facing errors Jan 19, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 19, 2026

🤖 Claude Code Review

Status: Complete

Current Review:
No issues found. This PR successfully implements error sanitization for user-facing services with:

  • Comprehensive new error helpers (UserMessage, PublicError, WrapGRPCPublic, WrapPublic) that strip internal details while preserving error semantics
  • Proper application across propagation, asset, and P2P services
  • Extensive test coverage including security scenarios
  • Safe concurrency patterns in error collection

History:

  • ✅ Fixed: Previous race condition concern on errStr was resolved by switching to slice-based collection with proper WaitGroup synchronization
  • ✅ Verified: Comprehensive test coverage added for all sanitization functions
  • ✅ Confirmed: P2P service correctly applies sanitization to user-facing endpoints

@gokutheengineer gokutheengineer changed the title feat: clenaup error messages for user facing errors feat: Error sanitization for user-facing services Jan 19, 2026
@gokutheengineer gokutheengineer requested review from icellan and removed request for icellan January 27, 2026 08:47
@gokutheengineer gokutheengineer force-pushed the gokhan/concat-error-msgs branch from 9b0916e to 0f870c1 Compare January 27, 2026 13:30
@gokutheengineer gokutheengineer force-pushed the gokhan/concat-error-msgs branch from 0e64ada to 05297aa Compare January 28, 2026 13:12
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 1, 2026

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant