MessageRequest callbacks never triggered if disconnected about avs-device-sdk HOT 7 CLOSED

@jshlt - No ETA at the moment. We will let you know as soon as we have something available.

from avs-device-sdk.

Hello jshlt,
Thank you for bringing this to our attention! We will investigate and get back to you.
-SWH

from avs-device-sdk.

Hello again jshlt,
I have NOT verified the problems that you are seeing, but there are a couple of known issues in MessageRouter::send() that may be the same or closely related:

1. The lambda function is not acquiring m_connectionMutex before using m_activeTransport. see Issue 5
2. The lambda function is missing an else clause to signal onSendCompleted(NOT_CONNECTED)
   We are preparing a fix to MessageRouter.cpp for both in the next update, along the following lines:

void MessageRouter::send(std::shared_ptr<avsCommon::avs::MessageRequest> request) {
    auto task = [this, request]() {
        std::shared_ptr<TransportInterface> transport;
        {
            std::lock_guard<std::mutex> lock{m_connectionMutex};
            transport = m_activeTransport;
        }
        if (transport && transport->isConnected()) {
            transport->send(request);
        } else {
            request->onSendCompleted(avsCommon::avs::MessageRequest::Status::NOT_CONNECTED);
        }
    };
    m_sendExecutor->submit(task);
}

Please let us know if this resolves the issue for you,
-SWH

from avs-device-sdk.

Hello jshslt,

Sadly, I have to report that I am able to reproduce the problem you describe both with and without the change I suggested above. Digging in to this it looks like there may be multiple problems.

One of these is that AVSConnectionManager::send() / MessageRequest level there is no interface for specifying a timeout for sending a message. Since AVSConnectionManager handles reconnecting to AVS internally, MessageRequests will only very rarely receive onSendCompleted() with the NOT_CONNECTED status. Instead they are left in limbo indefinitely, as you have observed.

We are actively working on this and will let you know when we have a fix.

Thank you again for bringing this to our attention!
-SWH

from avs-device-sdk.

@scotthea-amazon - thanks - just wanted to check in and see if you might know an ETA on a fix for this?

from avs-device-sdk.

Hello jshlt,

Changes were pushed in the 0.5 release that address most of the points you raised in this issue. The primary change are:

• When a disconnect is detected, onSendCompleted() notifications will be sent to all outstanding MessageRequests.
• When the request/reply of a message does not progress, onSendCompeted() will be called with a TIMEDOUT status.

An outstanding issue is that detection of the disconnected state can be quite slow. libcurl does not immediately detect an unconnected network cable and a request will not fail until the operation has timed out. This is discussed in more detail on the libcurl website here.

Please let us know how this update works for you,
-SWH

from avs-device-sdk.

Closing the issue for now. Please re-open if the problem still exists.

from avs-device-sdk.