Skip to content

Classify SQL execution errors for logging - #5869

Open
mahmudsudo wants to merge 4 commits into
clockworklabs:masterfrom
mahmudsudo:sql_errors
Open

Classify SQL execution errors for logging#5869
mahmudsudo wants to merge 4 commits into
clockworklabs:masterfrom
mahmudsudo:sql_errors

Conversation

@mahmudsudo

Copy link
Copy Markdown

(closes #5690 )
Summary:
Classifies errors returned from SQL execution so client-facing SQL errors are logged at warn and returned as 400 Bad Request, while internal datastore/durability/view failures are logged at error and return 500 Internal Server Error (without leaking internal details).

What I changed:

  • Files: lib.rs
  • Behavior: Host::exec_sql now matches on DBError variants from sql::execute::run:
    • Client/SQL errors (parser, plan/type/with-sql, common validation errors) are logged at warn and map to 400 Bad Request with the original message.
    • Internal failures (datastore I/O, durability, snapshots, unexpected internal view errors, etc.) are logged at error and map to 500 Internal Server Error with a generic message.
  • Rationale: preserve useful client feedback for user SQL mistakes while surfacing actionable logs for real internal failures.

Testing:

  • Built the client API crate locally: cargo build -p spacetimedb-client-api (build succeeded).

@JasonAtClockwork JasonAtClockwork left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mahmudsudo Thanks for taking this one on! Similar to #5855 we need structured errors from the execution layer that preserve whether the failure is client-caused or internal.

With Plan and WithSql, for example, they can wrap internal DBErrors, so treating them as client errors will be incorrect. At the same time the DBError::Other can be authorization failures which would fall out to 500/errors with this change.

This also does not compile due to a missing closing brace in the test code.

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.

Classify SQL execution errors for logging

3 participants