Credentials Stuck in "Request Sent" State When Issuing Out-of-Band Credentials with New Connections

[sagarkhole4]

When issuing credentials using the out-of-band protocol with connection reuse disabled or when establishing a new connection, the connection gets successfully established, but the credentials remain in the request-sent state indefinitely.

This issue does not occur when reusing an existing connection. In such cases, the credential issuance process transitions to the appropriate final state as expected.

---

Solution

To resolve the issue, we updated the code in the following file:

File: /core/build/modules/oob/OutOfBandApi.js

Original Code:

this.connectionsApi
    .returnWhenIsConnected(connectionRecord.id, { timeoutMs })
    .then((connectionRecord) => 
        this.emitWithConnection(outOfBandRecord, connectionRecord, messages)
    )
    .catch((error) => {
        if (error instanceof rxjs_1.EmptyError) {
            this.logger.warn(
                `Agent unsubscribed before connection got into ${connections_1.DidExchangeState.Completed} state`,
                error
            );
        } else {
            this.logger.error('Promise waiting for the connection to be complete failed.', error);
        }
    });

Updated Code:

try {
    connectionRecord = await this.connectionsApi.returnWhenIsConnected(connectionRecord.id, { timeoutMs });
    await this.emitWithConnection(outOfBandRecord, connectionRecord, messages);
} catch (error) {
    if (error instanceof rxjs_1.EmptyError) {
        this.logger.warn(
            `Agent unsubscribed before connection got into ${connections_1.DidExchangeState.Completed} state`,
            error
        );
    } else {
        this.logger.error('Promise waiting for the connection to be complete failed.', error);
    }
}

---

Why the Change Was Needed

The previous implementation used .then() and .catch() to handle the promise returned by returnWhenIsConnected. While functional, it caused issues in managing asynchronous operations and awaiting the completion of the connection process.

The updated implementation leverages **async/await** within a **try-catch** block, which provides:

• Improved Readability: Cleaner and more synchronous-looking code flow.
• Better Error Handling: Ensures exceptions are caught properly during the asynchronous operations.
• Reduced Risk: Avoids potential race conditions or unhandled promise rejections.

[TimoGlastra]

Thanks for opening the elaborate issue @sagarkhole4. could you explain a bit more why this fixes the issue?

[sagarkhole4]

Thank you for looking into this!

If we look at this code:

async withTenantAgent(options, withTenantAgentCallback) {
    this.logger.debug(`Getting tenant agent for tenant '${options.tenantId}' in with tenant agent callback`);
    const tenantAgent = await this.getTenantAgent(options);
    try {
        this.logger.debug(`Calling tenant agent callback for tenant '${options.tenantId}'`);
        const result = await withTenantAgentCallback(tenantAgent);
        return result;
    }
    catch (error) {
        this.logger.error(`Error in tenant agent callback for tenant '${options.tenantId}'`, { error });
        throw error;
    }
    finally {
        this.logger.debug(`Ending tenant agent session for tenant '${options.tenantId}'`);
        await tenantAgent.endSession();
    }
}

The issue here is that we are closing the tenant session in the finally block. This means that when the function returns before the connection process is completed, the wallet is closed. This led to errors, as the connection was not properly finalized before the session ended.

[sagarkhole4]

Hi @TimoGlastra , Do you suggest that we should update this code.

[genaris]

from [#717] (https://github.com/openwallet-foundation/credo-ts/pull/717#discussion_r857744050)

Well said, young(er) @TimoGlastra !

At the time this code was originally written, there was no timeoutMs option, so I think the reason why this then/catch block was used had to do with the goal of not blocking forever in case that the other party is unreachable or it is offline.

Now that there is a timeout parameter (set by default), it could be safer to await the connection to be completed before returning (as in the code @sagarkhole4 suggested), but I'm not sure if it will work completely for all use cases:

• In some flows, it is good to avoid blocking until DID Exchange messages are transmitted, so we can subscribe to events related to the OutOfBandRecord or ConnectionRecord and react when they arrive. A change like this would break this pattern
• Although emitWithConnection ensures that all AgentMessageReceivedEvent will be emitted, we cannot be completely certain that these messages will be effectively processed before reaching the finally block in withTenantAgent and closing the wallet

Probably, a practical way of solving this issue without touching anything in Credo's source code would be to put some more logic in your withTenantAgentCallback function: for instance, you can block until you get the outcome you expect from the protocol you are executing. In this case, if it's the Issue Credential protocol, you can wait until you get a particular CredentialStateChanged event (or a timeout occurs), and then (and only then) return (and end session as a result of executing the finally block).