Skip to content

fix(h2): log SET SCHEMA failure instead of swallowing - #2155

Open
Aias00 wants to merge 1 commit into
OtterMind:mainfrom
Aias00:fix/2149-h2dbmanager-empty-catch
Open

fix(h2): log SET SCHEMA failure instead of swallowing#2155
Aias00 wants to merge 1 commit into
OtterMind:mainfrom
Aias00:fix/2149-h2dbmanager-empty-catch

Conversation

@Aias00

@Aias00 Aias00 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #2149

Summary

H2DBManager had an empty catch around SET SCHEMA execution, silently swallowing SQLException failures with no log. Added @slf4j and log.error, mirroring sibling DBManagers (OscarDBManager, OracleDBManager, etc.).

Verification

  • mvn compile -> BUILD SUCCESS.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: The fix, verification, and PR description were produced with Claude Code assistance.

Copilot AI review requested due to automatic review settings July 26, 2026 06:46
@Aias00
Aias00 requested a review from openai0229 as a code owner July 26, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@openai0229

Copy link
Copy Markdown
Contributor

This is a useful low-risk logging improvement, but the current PR is not ready to merge without validating the exception path it changes.

Please add a focused H2DBManager test where Connection.setSchema throws SQLException and verify that connectDatabase contains the failure without propagating it. If the project test setup supports log capture, assert the schema name and exception are logged; otherwise document why the log itself is not asserted. Please rebase onto the latest main; #2177 also modifies H2DBManager, so whichever PR lands second must resolve that overlap.

@openai0229 openai0229 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.

The changed SQLException path still lacks a focused H2DBManager regression test, and this branch overlaps open PR 2177. The current head does not address the detailed review comment already posted, so this remains blocked pending an updated commit and re-review.

H2DBManager had an empty catch around SET SCHEMA execution, silently
swallowing failures. Add @slf4j and log.error, mirroring sibling
DBManagers.

Fixes OtterMind#2149

Co-Authored-By: Claude <noreply@anthropic.com>
@Aias00

Aias00 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main. Added 2 regression tests in H2DBManagerTest:

  1. connectDatabaseCatchesSQLExceptionForBadSchema — sets a non-existent schema name, verifies connectDatabase catches the SQLException (log shows: 'Failed to set schema: NONEXISTENT_SCHEMA_12345') and does not propagate it.
  2. connectDatabaseReturnsEarlyWhenSchemaNameIsBlank — verifies the early-return path when schemaName is blank.

Tests run: 2, Failures: 0. The log output confirms the schema name and exception are logged via log.error.

@Aias00
Aias00 force-pushed the fix/2149-h2dbmanager-empty-catch branch from 937f221 to f5b8895 Compare July 28, 2026 13:43
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.

H2DBManager swallows SET SCHEMA SQLException in an empty catch

3 participants