Improve error message when query is canceled during streaming #507
Conversation
This file contains 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
When streaming a result in single row mode, if this query is aborted - either because it timed out or some other reason -, `ruby-pg` raises the error: ``` PG::InvalidChangeOfResultFields: number of fields changed in single row mode from X to 0 - this is a sign for intersection with another query ``` This error message is misleading and might confuse people. This commit adds a different error message, using the same exception class, explaining that the query was canceled or timed out.
|
My initial thought would be to raise the original error from the database, but I couldn't find a way to do that. I'm not very familiar with C code, though. I'd appreciate help doing that. |
larskanis
added a commit
to larskanis/ruby-pg
that referenced
this pull request
Mar 21, 2023
This ensures that the nfield-check doesn't hide errors like statement timeout. Now the new PGresult is assigned to the streaming PG::Result, even if it has a different number of fields. This avoids leaking the memory of the wrong PGresult, but would lead to wrong fields, if the PG::Result is used afterwards. Fixes ged#507
larskanis
added a commit
to larskanis/ruby-pg
that referenced
this pull request
Mar 21, 2023
This ensures that the nfield-check doesn't hide errors like statement timeout. Now the new PGresult is assigned to the streaming PG::Result, even if it has a different number of fields. This avoids leaking the memory of the wrong PGresult, but would lead to wrong fields or segfaults, if the PG::Result is used afterwards. Therefore the result is getting cleared before the raise. Fixes ged#507
larskanis
added a commit
to larskanis/ruby-pg
that referenced
this pull request
Mar 21, 2023
This ensures that the nfield-check doesn't hide errors like statement timeout. Now the new PGresult is assigned to the streaming PG::Result, even if it has a different number of fields. This avoids leaking the memory of the wrong PGresult, but would lead to wrong fields or segfaults, if the PG::Result is used afterwards. Therefore the result is getting cleared before the raise. Thanks to Neil Carvalho @neilvcarvalho for the test case! Fixes ged#507
|
I opened #510 to fix this issue. It raises the original exception. |
|
Thank you, @larskanis! That was a good async pairing session 😁 I'm closing this PR in favor of #510 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When streaming a result in single row mode, if this query is aborted - either because it timed out or some other reason -,
ruby-pgraises the error:This error message is misleading and might confuse people. The number of fields changed to 0 because the result stopped being streamed.
This commit adds a different error message using the same exception class, explaining that the query was canceled or timed out.