Azure / Azure/iot-sdks-e2e-fx

C wrapper: connect2/disconnect2 are no-op stubs that return 200, then EnableTwin hangs forever

Open
#437 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
6
Forks
19
Avg merge
1d 6h
Merged PRs (30d)
10

Description

## Summary

The C wrapper's `connect2` and `disconnect2` REST handlers are unmodified swagger-generated stubs. They return `200 OK` without doing anything, so a caller believes the client reconnected when it did not. A subsequent `enableTwin` then blocks forever on an unbounded `std::condition_variable::wait`, wedging the wrapper until the test client's 150s read timeout fires.

This makes `test_twin_desired_props_patch` fail intermittently in the `c_*_edgehub_module` suites, e.g. horton-gate-build [161937](https://dev.azure.com/azure-iot-sdks/azure-iot-sdks/_build/results?buildId=161937).

## Detail

`test-runner/twin_tests.py` has a recovery path for a desired-property patch that does not arrive in the first wait window: `disconnect2()` -> `connect2_with_retry()` -> `enable_twin()`, to force EdgeHub to rebuild its cloud proxy.

Both handlers are pure stubs:

`docker_images/c/wrapper/generated/ModuleApi.cpp:193` (`connect2`) and `:532` (`disconnect2`):

```cpp
// Change the value of this variable to the appropriate response before sending the response
int status_code = 200;

/**
* Process the received information here
*/

if (status_code == 200) {
session->close(200, "", { {"Connection", "close"} });
return;
}
```

There is no corresponding `Connect2`/`Disconnect2` in `glue/InternalGlue.h`, `InternalGlue.cpp` or `ModuleGlue.cpp` at all.

Then `InternalGlue::EnableTwin` waits with no timeout — `glue/InternalGlue.cpp:212-216`:

```cpp
std::cout << "waiting for initial Twin response" << std::endl;
{
std::unique_lock lk(resp->m);
resp->cv.wait(lk, [resp]{ return !resp->latest_payload.empty(); });
}
```

Because the client was never actually reopened, no MQTT traffic is generated and `latest_payload` is never populated, so this never returns.

## Evidence from build 161937

`testMod.log` for `test_linux_amd64_c_mqttws_edgehub_module`. Note that the `disconnect2` and `connect2` routes are received but produce **no** `InternalGlue::` log line, unlike every other route:

```
{"message": "PYTEST: patch 3 not received after 45s"}
{"message": "PYTEST: patch 3 not received, reconnecting to rebuild EdgeHub cloud proxy"}
RESTBED:INFO: Incoming 'PUT' request ... for route '/module/moduleClient_28/disconnect2'.
RESTBED:INFO: Incoming 'PUT' request ... for route '/module/moduleClient_28/connect2'.
RESTBED:INFO: Incoming 'PUT' request ... for route '/module/moduleClient_28/enableTwin'.
InternalGlue::EnableTwin for moduleClient_28
waiting for initial Twin response
{"message": "PYTEST: TEST FAILED BACAUSE OF ... Read timed out. (read timeout=150)"}
```

For contrast, the *first* `enableTwin` on the same connection logs a full CONNECT/CONNACK/SUBSCRIBE/`\/twin/GET` exchange and returns immediately. After the fake reconnect there is no MQTT traffic at all.

The wrapper stays wedged afterwards, so the following test also fails on setup:

```
failed on setup with "... Max retries exceeded with url: /module/connectFromEnvironment/mqttws
(Caused by ReadTimeoutError(... read timeout=150))"
```

## Suggested fix

1. Implement `Connect2`/`Disconnect2` in the C glue and call them from the two generated handlers. Semantics, per the node and python wrappers: `disconnect2` closes the client but keeps it in `clientMap` under the same `connectionId`; `connect2` reopens that same client. Note a pending `WaitForDesiredPropertyPatch` must survive the cycle.
2. Bound the wait in `EnableTwin` (and the other `cv.wait` call sites) with `wait_for`, throwing on expiry. Even once (1) is fixed, an unbounded wait turns any lost twin response into a wedged wrapper and an opaque client-side timeout instead of a clear error.

Azure/azure-iot-sdk-java#1859 was the same class of bug in the Java SDK, and Azure/iot-sdks-e2e-fx#436 fixes the equivalent gap in the Java wrapper, where `moduleConnect2`/`moduleDisconnect2` threw `UnsupportedOperationException`. The C wrapper is arguably worse because it reports success rather than failing loudly.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with docker_images/c/wrapper/generated/ModuleApi.cpp at connect2 (line 193) and disconnect2 (line 532), then compare glue/InternalGlue.h, glue/InternalGlue.cpp, and glue/ModuleGlue.cpp with the node and Python wrapper semantics described. Run test-runner/twin_tests.py, including test_twin_desired_props_patch in the c_*_edgehub_module suites; done means reconnect routes perform the documented client lifecycle and EnableTwin exits clearly when no Twin response arrives.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
api, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.