🐛 Setting save_exchange_record=False does not promptly delete the record after exchange is complete

[ff137]

At the moment, this is only an issue for our tests. Namely: app/tests/e2e/issuer/test_save_exchange_record.py::test_issue_credential_with_save_exchange_record

In this e2e test we validate that a 404 is raised when trying to fetch the credential exchange record, after the exchange was complete, and if save_exchange_record was set to False.

Previously, we've identified some race condition, where it may take a moment for the records to update. Therefore, a naive workaround was to add a sleep of 1 second. This is fairly "long" in computational terms, so it was thought to be sufficient.

This did make the test failure rare, but it can still occur -- i.e. it can happen that the record has still not been deleted, 1 second after the exchange was complete.

This should be fine for most client use cases, since a short delay is acceptable. But all we can know from the test failure is that it took more than 1 second, and so we can't guarantee that the bug is not actually that it never gets deleted ...

A possible solution would be to replace the 1 second sleep with some fast retry logic, so it can pass as quickly as possible, and only fails if it takes, let's say, more than 5 seconds.

[ff137]

Sample test error logs:

_ test_issue_credential_with_save_exchange_record[clean-clean-clean-clean-clean-v1-False] _

faber_client = <shared.util.rich_async_client.RichAsyncClient object at 0x7fedaa5c3410>
credential_definition_id = 'X73s2fEXJnxSnwtX3VDp9S:3:CL:8478:tag'
faber_and_alice_connection = FaberAliceConnect(alice_connection_id='326fb1a7-7246-40f2-90f9-ce96d8497ba4', faber_connection_id='f350ed60-70f0-4593-b84b-33b9660a9f11')
alice_member_client = <shared.util.rich_async_client.RichAsyncClient object at 0x7fedaa594e60>
save_exchange_record = False, protocol_version = 'v1'

    @pytest.mark.anyio
    @pytest.mark.parametrize("save_exchange_record", [False, True])
    @pytest.mark.parametrize("protocol_version", ["v1", "v2"])
    async def test_issue_credential_with_save_exchange_record(
        faber_client: RichAsyncClient,
        credential_definition_id: str,
        faber_and_alice_connection: FaberAliceConnect,
        alice_member_client: RichAsyncClient,
        save_exchange_record: bool,
        protocol_version: str,
    ) -> CredentialExchange:
        credential = {
            "protocol_version": protocol_version,
            "connection_id": faber_and_alice_connection.faber_connection_id,
            "indy_credential_detail": {
                "credential_definition_id": credential_definition_id,
                "attributes": sample_credential_attributes,
            },
            "save_exchange_record": save_exchange_record,
        }
    
        # create and send credential offer- issuer
        faber_send_response = (
            await faber_client.post(
                CREDENTIALS_BASE_PATH,
                json=credential,
            )
        ).json()
    
        faber_credential_exchange_id = faber_send_response["credential_exchange_id"]
        thread_id = faber_send_response["thread_id"]
    
        payload = await check_webhook_state(
            client=alice_member_client,
            topic="credentials",
            state="offer-received",
            filter_map={
                "thread_id": thread_id,
            },
        )
    
        alice_credential_exchange_id = payload["credential_exchange_id"]
    
        # send credential request - holder
        await alice_member_client.post(
            f"{CREDENTIALS_BASE_PATH}/{alice_credential_exchange_id}/request",
        )
    
        await check_webhook_state(
            client=alice_member_client,
            topic="credentials",
            state="done",
            filter_map={
                "credential_exchange_id": alice_credential_exchange_id,
            },
        )
    
        time.sleep(1)  # short sleep before fetching cred ex records; allow them to update
    
        # faber requesting auto_remove only removes their cred ex records
        # get exchange record from alice side -- should not exist after complete
        with pytest.raises(HTTPException) as exc:
            await alice_member_client.get(
                f"{CREDENTIALS_BASE_PATH}/{alice_credential_exchange_id}"
            )
        assert exc.value.status_code == 404
    
        if save_exchange_record:
            # get exchange records from faber side:
            faber_cred_ex_record = (
                await faber_client.get(
                    f"{CREDENTIALS_BASE_PATH}/{faber_credential_exchange_id}"
                )
            ).json()
    
            # Save record True, should be 1 record
            assert (
                faber_cred_ex_record["credential_exchange_id"]
                == faber_credential_exchange_id
            )
    
            # Clean up
            await faber_client.delete(
                f"{CREDENTIALS_BASE_PATH}/{faber_credential_exchange_id}"
            )
        else:
            # If save_exchange_record was not set, credential should not exist
>           with pytest.raises(HTTPException) as exc:
E           Failed: DID NOT RAISE <class 'fastapi.exceptions.HTTPException'>

app/tests/e2e/issuer/test_save_exchange_record.py:105: Failed

Occurred most recently here: https://github.com/didx-xyz/aries-cloudapi-python/actions/runs/10272955775/job/28426557267