Daemon thread of TryRecv does not give notifications when errors occur

[soulzhao]

in AmsConnection::TryRecv the following code snippet:

try {
        Recv();
    } catch (const std::runtime_error& e) {
        LOG_INFO(e.what());
    }

it just eat up the runtime_error and didn't offer an way to notify the above callers, I think it would be better to enable the above callers to register hooks on this daemon thread so that they could monitor the connection status. I have a temporary fix like below:

try {
        Recv();
    } catch (const std::runtime_error& e) {
        LOG_INFO(e.what());
        if (sysHooksList.find(AMS_CONNECTION_LOST) != sysHooksList.end()) {
            PAdsEventNotificationFuncEx pFunc = sysHooksList.at(AMS_CONNECTION_LOST);
            pFunc(this->destIp.value, AMS_CONNECTION_LOST);
        }
    }

where std::map<uint32_t, PAdsEventNotificationFuncEx> sysHooksList; is a private member of AmsConnection to help register AmsConnection lifecycle hooks. I know you guys are smart enough, you can take this into consideration for the next commit.

[soulzhao]

Besides, I think in Sockets.cpp line 117,

if ((0 == bytesRead) || (lastError == CONNECTION_CLOSED) || (lastError == CONNECTION_ABORTED)) {

we could also add (lastError == WSAECONNRESET) to check whether the connection is lost or not

[pbruenn]

I'm a little unhappy with TryRecv(), too. I believe the unknown connection status might be the biggest issue with AdsLib at the moment and I definitely want to fix that. Your callback idea sounds good and I will keep that in mind for the time, when I try to tackle this connection status issue. Unfortunately this will not happen this year. So I will keep your issue open for a few weeks. Thanks for your feedback, Patrick

[pogojotz]

We have the same issue. Using the TcAdsDll, we used AdsAmsRegisterRouterNotification to observe the run state of the router. With AdsLib we just get a disconnect without a notification.

Would be nice, if AdsAmsRegisterRouterNotification could just be re-implemented in AdsLib, observing the remote router. Unfortunately that wouldn't work because we can connect to multiple remote routers and the callback wouldn't know, which router "stopped" (meaning disconnected). So either the signature of AdsAmsRegisterRouterNotification needed to be changed or the signature of the registered callback function to identify the route.

Would be great to see this resolved :)

[mriepl]

would be nice to fix this issue, i also have the problem that the lib doesnt send any information about the connection status.

[MarkRiddoch]

I would like to add my voice to the list of people that would like to see a callback for connection failures. We are using the library to monitor machines and suffer issues when the data stream just stops.

[soulzhao]

I would like to add my voice to the list of people that would like to see a callback for connection failures. We are using the library to monitor machines and suffer issues when the data stream just stops.

Try my solution and see if it helps