chore: remove all asserts in source code, replace with error for internal exceptions, and InvalidDirectiveExceptions for input errors #765
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
…rnal exceptions, and InvalidDirectiveExceptions for input errors
alharris-at
force-pushed
the
replace-asserts
branch
from
b0b7693
to
9a4f620
Compare
Codecov Report
@@ Coverage Diff @@
## main #765 +/- ##
==========================================
+ Coverage 63.14% 63.39% +0.24%
==========================================
Files 278 278
Lines 18125 18220 +95
Branches 4375 4431 +56
==========================================
+ Hits 11445 11550 +105
+ Misses 6093 6074 -19
- Partials 587 596 +9
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
phani-srikar
approved these changes
Aug 25, 2022
| * @param ctx the cli invocation context | ||
| * @returns the constructed sort field, or null if no sort field can be created | ||
| */ | ||
| export const tryAndCreateSortField = ( |
There was a problem hiding this comment.
appreciate this refactor!
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.
Description of changes
Today, we have a number of
assertstatements in source which we shouldn't do. This PR replaces those with eitherErrors for faults, orInvalidDirectiveExceptionswhere invalid input is caught, to provide better visibility into operator error vs bugs.Issue #, if available
N/A
Description of how you validated changes
I would have like to add more unit tests, but given the surface area of the change that was slowing me down, so I'm relying on e2e tests to verify all use-cases here.
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.