Skip to content
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

EDP Simulator Should Handle Requisitions in the Withdrawn State #1903

Open
kungfucraig opened this issue Nov 6, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@kungfucraig
Copy link
Member

Recently, a Withdrawn state was added for Requisitions. The EDP Simulator has not been updated to handle this case.

The correct behavior is for the EDP Simulator to handle an Illegal State exception during fulfillment by calling "Get" and then ignoring the exception if the state of the Requisition is withdrawn. Otherwise the exception should be propagated.

Once this issue is fixed, it can serve as an example of how a real EDP should handle this situation.

@kungfucraig kungfucraig added the bug Something isn't working label Nov 6, 2024
@SanjayVas
Copy link
Member

SanjayVas commented Nov 13, 2024

The correct behavior is for the EDP Simulator to handle an Illegal State exception during fulfillment by calling "Get" and then ignoring the exception if the state of the Requisition is withdrawn. Otherwise the exception should be propagated.

This is not actually the correct behavior, as the caller (EDP simulator) cannot actually determine that the FAILED_PRECONDITION error is due to Requisition state mismatch. The correct behavior would be one of the following:

  1. Inspecting the ErrorInfo (see AIP-193) from the Fulfill RPC to see the state.

    This requires plumbing the error info properly through from the Kingdom internal API to Kingdom system API to the Duchy public API, as this primarily happens when fulfilling at the Duchy. This is something that we should eventually do to conform with the AIPs. We currently only plumb ErrorInfo from the Kingdom internal API to the Kingdom public API. Furthermore, that ErrorInfo plumbing is incorrect (see ErrorInfo from Kingdom public API malformed #1919).

  2. Passing a Requisition etag in the fulfill request and handling ABORTED errors by retrying at a higher level, i.e. calling Get and checking the state

    This requires the API definition to include an etag field in Requisition and the services to implement AIP-154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants