fix: Ensure intra-cluster InternalExceptions are propagated properly #58
Conversation
|
Hi @njhill -- Your changes look good as far as I understand from reading the code. But would you have an example use case for me to test this before and after your change? |
|
@ckadner good question... it's kind of an edge case but will try to craft a unit test to cover it. |
Motivation In certain error edge cases, InternalExceptions can be thrown on the inferencing path, but these currently don't propagate properly because they aren't declared to be thrown by the internal thrift applyModel(Multi) method. Because of this, they are treated as generic errors meaning that the error detail (message) is lost and when returned to the user can have a confusing message like "Nowhere available to load". Modifications - Add InternalException to internal applyModel thrift rpc method definitions - Also update to use latest version of thrift Result InternalExceptions will propagate properly between modelmesh pods Signed-off-by: Nick Hill <[email protected]>
Signed-off-by: Nick Hill <[email protected]>
|
@ckadner have added a unit test and also fixed a related bug that it exposed, PTAL! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ckadner, njhill The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Sorry for the delay @njhill -- I ran the unit tests including running your new test without your code changes to verify they would fail. I also built a new |
Create Workflow for release and tag with Changelog
Motivation
In certain error edge cases,
InternalExceptions can be thrown on the inferencing path, but these currently don't propagate properly because they aren't declared to be thrown by the internal thriftapplyModel(Multi)method. Because of this, they are treated as generic errors meaning that the error detail (message) is lost and when returned to the user can have a confusing message like "Nowhere available to load".Furthermore, there was a bug when checking the type of remote exceptions, where in certain places they were not first being unwrapped as intended.
Modifications
InternalExceptionto internalapplyModelthrift rpc method definitionsInvocationTargetExceptionfirstResult
InternalExceptions will propagate properly between modelmesh pods