push ping time only if the request complete is successful (#348)

source/client_channel_handler.c

@@ -1032,7 +1032,9 @@ void mqtt_request_complete(struct aws_mqtt_client_connection_311_impl *connectio
             aws_mqtt_connection_statistics_change_operation_statistic_state(
                 request->connection, request, AWS_MQTT_OSS_NONE);
 
-            s_pushoff_next_ping_time(connection, request->request_send_timestamp);
+            if (error_code == AWS_OP_SUCCESS) {
+                s_pushoff_next_ping_time(connection, request->request_send_timestamp);
+            }
             /* clean up request resources */
             aws_hash_table_remove_element(&connection->synced_data.outstanding_requests_table, elem);
             /* remove the request from the list, which is thread_data.ongoing_requests_list */

tests/CMakeLists.txt

@@ -78,6 +78,7 @@ add_test_case(mqtt_clean_session_not_retry)
 add_test_case(mqtt_clean_session_discard_previous)
 add_test_case(mqtt_clean_session_keep_next_session)
 add_test_case(mqtt_connection_publish_QoS1_timeout)
+add_test_case(mqtt_connection_publish_QoS1_timeout_with_ping)
 add_test_case(mqtt_connection_unsub_timeout)
 add_test_case(mqtt_connection_publish_QoS1_timeout_connection_lost_reset_time)
 add_test_case(mqtt_connection_ping_norm)

tests/v3/connection_state_test.c

@@ -3015,6 +3015,79 @@ AWS_TEST_CASE_FIXTURE(
     s_clean_up_mqtt_server_fn,
     &test_data)
 
+/**
+ * Test that connection is healthy, user set the timeout for request, and timeout happens we still send ping reqs
+ */
+static int s_test_mqtt_connection_publish_QoS1_timeout_with_ping_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)allocator;
+    struct mqtt_connection_state_test *state_test_data = ctx;
+
+    struct aws_mqtt_connection_options connection_options = {
+        .user_data = state_test_data,
+        .clean_session = false,
+        .client_id = aws_byte_cursor_from_c_str("client1234"),
+        .host_name = aws_byte_cursor_from_c_str(state_test_data->endpoint.address),
+        .socket_options = &state_test_data->socket_options,
+        .on_connection_complete = s_on_connection_complete_fn,
+        .ping_timeout_ms = 99,
+        .protocol_operation_timeout_ms = 20,
+        .keep_alive_time_secs = 3, // connection->keep_alive_time_ns, pushoff the timestamp by that amount
+    };
+
+    struct aws_byte_cursor pub_topic = aws_byte_cursor_from_c_str("/test/topic");
+    struct aws_byte_cursor payload_1 = aws_byte_cursor_from_c_str("Test Message 1");
+
+    ASSERT_SUCCESS(aws_mqtt_client_connection_connect(state_test_data->mqtt_connection, &connection_options));
+    s_wait_for_connection_to_complete(state_test_data);
+
+    /* Disable the auto ACK packets sent by the server, which blocks the requests to complete */
+    mqtt_mock_server_disable_auto_ack(state_test_data->mock_server);
+
+    /* make a publish with QoS 1 immediate. */
+    aws_mutex_lock(&state_test_data->lock);
+    state_test_data->expected_ops_completed = 10;
+    aws_mutex_unlock(&state_test_data->lock);
+
+    for (int i = 0; i < 10; i++) {
+        uint16_t packet_id_1 = aws_mqtt_client_connection_publish(
+            state_test_data->mqtt_connection,
+            &pub_topic,
+            AWS_MQTT_QOS_AT_LEAST_ONCE,
+            false,
+            &payload_1,
+            s_on_op_complete,
+            state_test_data);
+        ASSERT_TRUE(packet_id_1 > 0);
+
+        // sleep 1 second
+        aws_thread_current_sleep(1000000000);
+    }
+
+    // Wait for 3 seconds
+    aws_thread_current_sleep(3000000000);
+
+    // make sure we are still receiveing pings when the connection is down, in other words ping pushoff is not happening
+    ASSERT_TRUE(mqtt_mock_server_get_ping_count(state_test_data->mock_server) > 1);
+
+    aws_channel_shutdown(state_test_data->server_channel, AWS_OP_SUCCESS);
+
+    /* publish should complete after the shutdown */
+    s_wait_for_ops_completed(state_test_data);
+
+    ASSERT_SUCCESS(
+        aws_mqtt_client_connection_disconnect(state_test_data->mqtt_connection, s_on_disconnect_fn, state_test_data));
+    s_wait_for_disconnect_to_complete(state_test_data);
+
+    return AWS_OP_SUCCESS;
+}
+
+AWS_TEST_CASE_FIXTURE(
+    mqtt_connection_publish_QoS1_timeout_with_ping,
+    s_setup_mqtt_server_fn,
+    s_test_mqtt_connection_publish_QoS1_timeout_with_ping_fn,
+    s_clean_up_mqtt_server_fn,
+    &test_data)
+
 /**
  * Test that connection is healthy, user set the timeout for request, and timeout happens and the unsubscribe failed.
  */