Skip to content

hotfix: revert BATCH_CONNECTION_REQUESTS default to False#18

Merged
agonza1 merged 1 commit into
peermetrics:masterfrom
Boanerges1996:fix/disable-broken-batch-connection-requests
Apr 8, 2026
Merged

hotfix: revert BATCH_CONNECTION_REQUESTS default to False#18
agonza1 merged 1 commit into
peermetrics:masterfrom
Boanerges1996:fix/disable-broken-batch-connection-requests

Conversation

@Boanerges1996
Copy link
Copy Markdown
Contributor

Summary

PR #13 changed BATCH_CONNECTION_REQUESTS default from False to True. This breaks connection monitoring because batching is broken at two levels:

SDK (client): addConnection calls sendConnectionEvent() and expects an immediate response with connection_id and peer_id. With batching enabled, the event is queued (returns undefined), the SDK throws "There was a problem while adding this connection", and calls removeConnection() — destroying the monitoring before it starts.

API (server): The batch endpoint (ConnectionEventBatchView.post()) returns an empty JSONHttpResponse(status=200) with no body. The non-batch endpoint returns { peer_id, connection_id } for addConnection events — the batch endpoint does not.

Fix

One-line change: default back to False.

Test plan

  • Deploy and verify SDK addConnection succeeds (no console errors)
  • Verify connection events appear in the PeerMetrics dashboard

PR peermetrics#13 changed the default to True, but batching is broken:

1. SDK: addConnection expects an immediate response with connection_id
   and peer_id. With batching, sendConnectionEvent queues the event and
   returns undefined, causing the SDK to throw and remove the connection.

2. API: the batch endpoint (ConnectionEventBatchView) returns an empty
   200 with no connection_id or peer_id, so even if the SDK didn't
   crash, it couldn't map subsequent events to the connection.

Reverting to False (original behavior) to prevent connection monitoring
from breaking in production.
@agonza1 agonza1 merged commit 804f9b2 into peermetrics:master Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants