-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RD-11521: report connection failures to the user #469
RD-11521: report connection failures to the user #469
Conversation
One comment is that we found a slightly different behaviour when doing LSP calls vs validate/execute calls. So we should indeed be sure for the behaviour we want for LSP. Probably it is indeed safe to ignore as you mention for the hover. |
1d09d9b
to
5923c2f
Compare
I added tests that trigger both errors for LSP calls, ensuring they return nothing silently. There was even a place missing the // hover returns nothing
val hoverResponse = runHover(compilerService, joe, "SELECT * FROM example.airports", Pos(1, 17))
assert(hoverResponse.completion.isEmpty)
val hoverResponse2 =
runHover(compilerService, joe, "SELECT * FROM example.airports WHERE :id = airport_id", Pos(1, 40))
assert(hoverResponse2.completion.isEmpty)
// we get no word completions
val wordCompletionResponse = runWordCompletion(compilerService, joe, "SELECT * FROM exa", "exa", Pos(1, 17))
assert(wordCompletionResponse.completions.isEmpty)
// we get no dot completions
val dotCompletionResponse = runDotCompletion(compilerService, joe, "SELECT * FROM example.", Pos(1, 22))
assert(dotCompletionResponse.completions.isEmpty) |
5923c2f
to
823815a
Compare
sql-client/src/main/scala/raw/client/sql/metadata/UserMetadataCache.scala
Outdated
Show resolved
Hide resolved
sql-client/src/main/scala/raw/client/sql/SqlCompilerService.scala
Outdated
Show resolved
Hide resolved
sql-client/src/main/scala/raw/client/sql/metadata/UserMetadataCache.scala
Outdated
Show resolved
Hide resolved
sql-client/src/test/scala/raw/client/sql/TestSqlConnectionFailures.scala
Outdated
Show resolved
Hide resolved
sql-client/src/test/scala/raw/client/sql/TestSqlConnectionFailures.scala
Outdated
Show resolved
Hide resolved
05eaaa6
to
6bd93ec
Compare
Before the pool was global, the code logic was that a connection failure to FDW would be an internal bug, FDW should always be available, and the connection pool should be provisioned to match the expected usage.
To review carefully if this behavior is what we want, and is implemented properly: To report connection failures to the user, we should isolate the connection failure
SQLException
(those are identified with code08xxx
), and process these ones as errors that are normal to hit sometimes:execute
,validate
andgetProgramDescription
, they are caught are the message is reported (as aString
orErrorMessage
with positions when needed). Thevalidate
andgetProgramDescription
need positions, I've put positions spanning the full code.hover
, we probably should ignore it and and do not report type info,The PR only adds tests to
execute
,validate
andgetProgramDescription
and LSP calls.