Clean up errors from key provider options parser #355
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (80.67%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## TDE_REL_17_STABLE #355 +/- ##
=====================================================
+ Coverage 80.49% 80.67% +0.18%
=====================================================
Files 22 22
Lines 2604 2598 -6
Branches 398 395 -3
=====================================================
Hits 2096 2096
+ Misses 430 427 -3
+ Partials 78 75 -3
🚀 New features to boost your workflow:
|
| ereport(ERROR, | ||
| errmsg("invalid semantic state")); | ||
| Assert(0); | ||
| ereport(ERROR, errmsg("invalid semantic state")); |
There was a problem hiding this comment.
Errors which cannot happen should either use elog or errmsg_internal.
There was a problem hiding this comment.
errmsg_internal isn't available in frontend iirc, so will go with elog then. Thanks!
There was a problem hiding this comment.
this has to do with everything in errmsg() being exported for translation, right? I don't think we'll ever bother with that, but ofc we should follow the standard from core.
This function could only ever return JSON_SUCCESS.
In the key provider options parser. To make the intent behind the code more obvious. Also use elog() for these errors to drive the point home.
To make the intent of the code clear without pretending we can return something else than JSON_SUCCESS.
Add error codes and make them more coherent with other error messages. Also add quotes around strings to make it clearer what the error actually is.
To give the same error messages as any other json parsing issue. Normally the values we get in here would already be valid postgres json values however, so finding a parsing error should be rare.
AndersAstrand
force-pushed
the
tde/clean-up-error-messages-from-options-parser
branch
from
1db9471 to
8be465d
Compare
I started this when doing other work with the options parser but never got it to PR then.