When endpoint is called multiple times then error is reported

[PaulZadorozhniy]

If the endpoint is called multiple times then error is reported /issue-credential-2.0/records/{cred_ex_id}/send-request

@docs(
    tags=["issue-credential v2.0"],
    summary="Send issuer a credential request",
)
@match_info_schema(V20CredExIdMatchInfoSchema())
@request_schema(V20CredRequestRequestSchema())
@response_schema(V20CredExRecordSchema(), 200, description="")
async def credential_exchange_send_bound_request(request: web.BaseRequest):
    """
    Request handler for sending credential request.

    Args:
        request: aiohttp request object

    Returns:
        The credential exchange record

    """
    r_time = get_timer()

    context: AdminRequestContext = request["context"]
    profile = context.profile
    outbound_handler = request["outbound_message_router"]

    try:
        body = await request.json() or {}
        holder_did = body.get("holder_did")
    except JSONDecodeError:
        holder_did = None

    cred_ex_id = request.match_info["cred_ex_id"]

    cred_ex_record = None
    conn_record = None
    try:
        async with profile.session() as session:
            try:
                cred_ex_record = await V20CredExRecord.retrieve_by_id(
                    session,
                    cred_ex_id,
                )
            except StorageNotFoundError as err:
                raise web.HTTPNotFound(reason=err.roll_up) from err

            connection_id = cred_ex_record.connection_id
            conn_record = await ConnRecord.retrieve_by_id(session, connection_id)

        if not conn_record.is_ready:
            raise web.HTTPForbidden(reason=f"Connection {connection_id} not ready")

        cred_manager = V20CredManager(profile)
        cred_ex_record, cred_request_message = await cred_manager.create_request(
            cred_ex_record,
            holder_did if holder_did else conn_record.my_did,
        )

        result = cred_ex_record.serialize()

    except (
        BaseModelError,
        IndyHolderError,
        LedgerError,
        StorageError,
        V20CredFormatError,
        V20CredManagerError,
    ) as err:
        LOGGER.exception("Error preparing bound credential request")
        if cred_ex_record:
            async with profile.session() as session:
                await cred_ex_record.save_error_state(session, reason=err.roll_up)
        await report_problem(
            err,
            ProblemReportReason.ISSUANCE_ABANDONED.value,
            web.HTTPBadRequest,
            cred_ex_record,
            outbound_handler,
        )

    await outbound_handler(cred_request_message, connection_id=connection_id)

    trace_event(
        context.settings,
        cred_request_message,
        outcome="credential_exchange_send_bound_request.END",
        perf_counter=r_time,
    )

    return web.json_response(result)

The issue is that whatever is thrown in the "try" is propagated as the problem report, even state checks.

 async def create_request(
        self, cred_ex_record: V20CredExRecord, holder_did: str, comment: str = None
    ) -> Tuple[V20CredExRecord, V20CredRequest]:
        """
        Create a credential request.

        Args:
            cred_ex_record: credential exchange record for which to create request
            holder_did: holder DID
            comment: optional human-readable comment to set in request message

        Returns:
            A tuple (credential exchange record, credential request message)

        """
        if cred_ex_record.cred_request:
            raise V20CredManagerError(
                "create_request() called multiple times for "
                f"v2.0 credential exchange {cred_ex_record.cred_ex_id}"
            )

        # react to credential offer, use offer formats
        if cred_ex_record.state:
            if cred_ex_record.state != V20CredExRecord.STATE_OFFER_RECEIVED:
                raise V20CredManagerError(
                    f"Credential exchange {cred_ex_record.cred_ex_id} "
                    f"in {cred_ex_record.state} state "
                    f"(must be {V20CredExRecord.STATE_OFFER_RECEIVED})"
                )

[swcurran]

What is the correction/action that needs to be taken because of this? It seems like calling that endpoint multiple times is not the normal flow, but what should happen when it is called multiple times? Is it OK just to see if the action has already been executed and if so, quietly return? Or should there be some indication that this is an unnecessary extra call?

Is this a general issue or specific to the identified endpoint?

[acuderman]

I think that the problem should not be reported but rather just an HTTP error with 4xx status returned. The issue is that at the moment we need to call the GET endpoint to check the status before the POST/PUT (action) calls as we do not want to override existing actions with a problem report (eg. We do not want to override credential-request status with abandoned). This causes performance degradation and possible race conditions. I also think the "problem report" part is more of a business logic thus more appropriate as part of a controller. As we want to increase the throughput of our system this would significantly improve the performance.

It is a general issue, not just on the identified endpoint

[swcurran]

So this is a similar example to #1798 -- when an endpoint is called subsequent time, ACA-Py is too aggressive at posting a problem report vs. just detecting that the call is a duplicate call and that's OK.

Could you do a PR for this specific instance of this issue as a way to get the ball rolling on getting more of this handling in place? I think this is going to have to be done on a protocol-by-protocol, state-by-state basis vs. some sort of "across the board" change. We can then debate how to get this done more broadly in ACA-Py.