{"action":"created","issue":{"url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp/issues/94","repository_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp","labels_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp/issues/94/labels{/name}","comments_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp/issues/94/comments","events_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp/issues/94/events","html_url":"https://github.com/aws/aws-iot-device-sdk-cpp/issues/94","id":316798785,"number":94,"title":"Subscribe does not remove subscriptions from map on failure","user":{"login":"shekharhimanshu","id":15043069,"avatar_url":"https://avatars2.githubusercontent.com/u/15043069?v=4","gravatar_id":"","url":"https://api.github.com/users/shekharhimanshu","html_url":"https://github.com/shekharhimanshu","followers_url":"https://api.github.com/users/shekharhimanshu/followers","following_url":"https://api.github.com/users/shekharhimanshu/following{/other_user}","gists_url":"https://api.github.com/users/shekharhimanshu/gists{/gist_id}","starred_url":"https://api.github.com/users/shekharhimanshu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shekharhimanshu/subscriptions","organizations_url":"https://api.github.com/users/shekharhimanshu/orgs","repos_url":"https://api.github.com/users/shekharhimanshu/repos","events_url":"https://api.github.com/users/shekharhimanshu/events{/privacy}","received_events_url":"https://api.github.com/users/shekharhimanshu/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-04-23T12:52:04Z","updated_at":"2018-05-01T01:51:45Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"Hello,\r\n\r\nIt looks like Subscribe api [inserts the subscription into the subscription map before sending the actual request](https://github.com/aws/aws-iot-device-sdk-cpp/blob/master/src/mqtt/Subscribe.cpp#L248...L267).\r\n\r\nIt then [waits for the SUBACK packet](https://github.com/aws/aws-iot-device-sdk-cpp/blob/master/src/ClientCoreState.cpp#L123).\r\n\r\nSubscribe gives up if say, SUBACK is not received within specified time and `ResponseCode::MQTT_REQUEST_TIMEOUT_ERROR` is returned to the user.\r\n\r\nHowever, the subscription map is left untouched. Ideally since subscribe to the topic was not successful, it should be removed from the map to avoid any side effects.\r\n\r\nOne such side effect is inside the Keep-Alive thread:\r\nThe Keep-Alive thread will [try to resubscribe to topics present in subscription map](https://github.com/aws/aws-iot-device-sdk-cpp/blob/master/src/mqtt/Connect.cpp#L480...L505) and hence will also retry those not successful subscriptions each time. In worst case scenario, reconnect might never succeed due to bad subscriptions present inside the subscription map.\r\n\r\nThanks."},"comment":{"url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp/issues/comments/385579783","html_url":"https://github.com/aws/aws-iot-device-sdk-cpp/issues/94#issuecomment-385579783","issue_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-cpp/issues/94","id":385579783,"user":{"login":"shekharhimanshu","id":15043069,"avatar_url":"https://avatars2.githubusercontent.com/u/15043069?v=4","gravatar_id":"","url":"https://api.github.com/users/shekharhimanshu","html_url":"https://github.com/shekharhimanshu","followers_url":"https://api.github.com/users/shekharhimanshu/followers","following_url":"https://api.github.com/users/shekharhimanshu/following{/other_user}","gists_url":"https://api.github.com/users/shekharhimanshu/gists{/gist_id}","starred_url":"https://api.github.com/users/shekharhimanshu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shekharhimanshu/subscriptions","organizations_url":"https://api.github.com/users/shekharhimanshu/orgs","repos_url":"https://api.github.com/users/shekharhimanshu/repos","events_url":"https://api.github.com/users/shekharhimanshu/events{/privacy}","received_events_url":"https://api.github.com/users/shekharhimanshu/received_events","type":"User","site_admin":false},"created_at":"2018-05-01T01:51:45Z","updated_at":"2018-05-01T01:51:45Z","author_association":"CONTRIBUTOR","body":">With reference to the API to remove subscriptions, it could either remove all subscriptions that failed the first time (the addition of the extra flag makes it easy), or remove specific subscriptions as you suggested.\r\n\r\nBoth of your proposal LGTM. Thanks."}}
{"actor":{"display_login":"shekharhimanshu"}}