fix: actionable error message in ExposedConnectionProvider#32
Merged
Conversation
When ExposedConnectionProvider.withConnection is called outside an active
Exposed transaction, TransactionManager.current() throws a generic Exposed
"No transaction in context" IllegalStateException that doesn't tell the
caller they are using okapi outside the expected transaction { } scope.
Wrap the lookup with TransactionManager.currentOrNull() and throw an
IllegalStateException whose message names the okapi method and points at
the missing Exposed transaction { } block. Test JdbcConnectionProvider
already does this same fail-fast on its ThreadLocal scope; this brings
ExposedConnectionProvider to parity.
Coverage:
* ExposedConnectionProviderTest — unit (FunSpec):
- throws IllegalStateException with actionable message when called
outside any transaction
- supplies the active transaction's connection to the block when
called inside transaction(db) { }
Follow-up to PR #26 review feedback (code-reviewer Suggestion #6).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ExposedConnectionProvider.withConnectionpreviously delegated toTransactionManager.current(), which throws a generic ExposedIllegalStateException("No transaction in context")when called outside a transaction. The message gives no hint about okapi.TransactionManager.currentOrNull()and throw a clearerIllegalStateExceptionnaming the okapi method and the missingtransaction { }block. BringsExposedConnectionProviderto parity with howJdbcConnectionProvider(test helper) reports its own missing-scope precondition.Exposed transaction { } blockhint), one for the positive path (asserts the active Exposed connection is supplied to the block).Motivation
Follow-up to PR #26 review feedback (
code-reviewerSuggestion #6 on the second review round). Three-line code change but it materially improves diagnostics for users who hit this misconfiguration in Ktor + Exposed setups.Test plan
./gradlew :okapi-exposed:test --tests "*ExposedConnectionProviderTest*"— both cases pass./gradlew build— full project build, all existing tests pass./gradlew ktlintFormat— no formatting issues