Skip to content

Fix timing issue: start WebRTC connection when clicking peer icon #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Abmarne wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix timing issue: start WebRTC connection when clicking peer icon #24

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 83 additions & 7 deletions client/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,17 @@ function addPeer(id, name) {
<div class="peer-name">${name}</div>
`;

// Click to send file
// Click to send file - start connection immediately when clicking on peer
el.addEventListener('click', () => {
currentTransferTarget = id;

// Start WebRTC connection immediately when user clicks on peer
// This gives time for connection to establish before file is selected
const peer = peers.get(id);
if (!peer.connection || peer.connection.connectionState !== 'connected') {
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clicking a peer can trigger startConnection(id) repeatedly (e.g., multiple clicks while connectionState is still new/connecting). startConnection always creates a new DataChannel and sends a new offer, which can lead to multiple channels/offers in flight and negotiation errors. Consider tracking a per-peer “connecting/negotiating” flag (or checking pc.signalingState / existing peer.dataChannel) so startConnection is only invoked once until it either connects or fails.

Suggested change
if (!peer.connection || peer.connection.connectionState !== 'connected') {
const state = peer && peer.connection ? peer.connection.connectionState : null;
if (!peer.connection || state === 'failed' || state === 'disconnected' || state === 'closed') {

Copilot uses AI. Check for mistakes.
startConnection(id);
}

fileInput.click();
});

Expand Down Expand Up @@ -180,6 +188,16 @@ function getOrCreateConnection(peerId) {
if (!peer.connection) {
const pc = new RTCPeerConnection(rtcConfig);

// Track connection state
pc.onconnectionstatechange = () => {
console.log(`Connection state with ${peer.name}: ${pc.connectionState}`);
if (pc.connectionState === 'connected') {
showToast(`Connected to ${peer.name}`, 'success');
} else if (pc.connectionState === 'failed' || pc.connectionState === 'disconnected') {
showToast(`Connection lost with ${peer.name}`, 'error');
}
};

// Output ICE candidates to signaling server
pc.onicecandidate = (e) => {
if (e.candidate) {
Expand Down Expand Up @@ -263,20 +281,78 @@ fileInput.addEventListener('change', async (e) => {
const file = e.target.files[0];
if (!file || !currentTransferTarget) return;

// If connection isn't established, establish it first
const peer = peers.get(currentTransferTarget);
if (!peer.connection || peer.connection.connectionState !== 'connected') {
await startConnection(currentTransferTarget);
// Wait briefly for connection (in reality, should listen for connection state change)
setTimeout(() => sendFileHeader(currentTransferTarget, file), 1000);
} else {

// Check if connection exists and is connected
if (peer.connection && peer.connection.connectionState === 'connected') {
sendFileHeader(currentTransferTarget, file);
Comment on lines 284 to 288
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileInput change handler assumes the selected currentTransferTarget still exists. If the peer leaves between click and file selection, peers.get(currentTransferTarget) will be undefined and peer.connection will throw. Add a guard for missing peer (and ideally reset currentTransferTarget / show a toast) before accessing peer.connection.

Copilot uses AI. Check for mistakes.
} else {
// Connection is being established (or was just started on peer click)
// Wait for connection to be ready with exponential backoff
let attempts = 0;
const maxAttempts = 10;
const checkConnection = () => {
attempts++;
if (peer.connection && peer.connection.connectionState === 'connected') {
sendFileHeader(currentTransferTarget, file);
} else if (attempts < maxAttempts) {
// Exponential backoff: 100ms, 200ms, 400ms, etc.
setTimeout(checkConnection, 100 * Math.pow(2, attempts));
Comment on lines +294 to +300
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exponential-backoff timeout calculation doesn’t match the comment and grows very large: with attempts++ before scheduling, the first delay is 200ms (not 100ms) and by attempt 10 it waits ~102s, so the total wait can exceed 3 minutes before failing. Consider using 2 ** (attempts - 1) (or increment after scheduling) and capping the delay to a reasonable max.

Suggested change
const checkConnection = () => {
attempts++;
if (peer.connection && peer.connection.connectionState === 'connected') {
sendFileHeader(currentTransferTarget, file);
} else if (attempts < maxAttempts) {
// Exponential backoff: 100ms, 200ms, 400ms, etc.
setTimeout(checkConnection, 100 * Math.pow(2, attempts));
const maxDelay = 5000; // cap per-attempt delay to 5 seconds
const checkConnection = () => {
attempts++;
if (peer.connection && peer.connection.connectionState === 'connected') {
sendFileHeader(currentTransferTarget, file);
} else if (attempts < maxAttempts) {
// Exponential backoff: 100ms, 200ms, 400ms, etc., capped at maxDelay
const delay = Math.min(100 * Math.pow(2, attempts - 1), maxDelay);
setTimeout(checkConnection, delay);

Copilot uses AI. Check for mistakes.
} else {
showToast('Connection failed. Try again.', 'error');
}
};
checkConnection();
}

fileInput.value = ''; // Reset input
});

function sendFileHeader(peerId, file) {
const peer = peers.get(peerId);
if (!peer || !peer.dataChannel) {
showToast('Connection not ready. Trying again...', 'info');

// Try to start connection and wait for it
startConnection(peerId);
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startConnection is async but is invoked here without await/.catch(...). If createOffer/setLocalDescription fails, this can surface as an unhandled promise rejection and you won’t be able to notify the user. Consider handling the returned promise (even if you don’t block UI) and showing an error toast on failure.

Suggested change
startConnection(peerId);
startConnection(peerId).catch((err) => {
console.error('Failed to start connection for peer', peerId, err);
showToast('Connection failed. Try again.', 'error');
});

Copilot uses AI. Check for mistakes.
let attempts = 0;
Comment on lines +312 to +318
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In sendFileHeader, the !peer and !peer.dataChannel cases are handled together, but the function still calls startConnection(peerId) even when peer is null/undefined. That will cause getOrCreateConnection to return null and then pc.createDataChannel(...) will throw. Split the condition so that a missing peer returns early without trying to connect.

Copilot uses AI. Check for mistakes.
const checkDataChannel = () => {
attempts++;
if (peer.dataChannel && peer.dataChannel.readyState === 'open') {
doSendFileHeader(peerId, file);
} else if (attempts < 10) {
setTimeout(checkDataChannel, 200);
} else {
showToast('Connection failed. Try again.', 'error');
}
};
setTimeout(checkDataChannel, 500);
return;
}

if (peer.dataChannel.readyState !== 'open') {
showToast('Connection not ready. Trying again...', 'info');

// Wait for data channel to open
let attempts = 0;
const checkDataChannel = () => {
attempts++;
if (peer.dataChannel.readyState === 'open') {
doSendFileHeader(peerId, file);
} else if (attempts < 10) {
setTimeout(checkDataChannel, 200);
} else {
showToast('Connection failed. Try again.', 'error');
}
};
setTimeout(checkDataChannel, 200);
return;
}

doSendFileHeader(peerId, file);
}

function doSendFileHeader(peerId, file) {
const peer = peers.get(peerId);
if (!peer || !peer.dataChannel || peer.dataChannel.readyState !== 'open') {
showToast('Connection not ready. Try again.', 'error');
Expand Down
Loading