Skip to content

fix: Implement basic error handling and user-friendly alerts #8

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
sebmuehlbauer wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Implement basic error handling and user-friendly alerts #8

Show file tree
Hide file tree
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
28 changes: 28 additions & 0 deletions client/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const alertBox = document.getElementById('alert-box');

function showAlert(message) {
alertBox.textContent = message;
alertBox.className = 'error';
alertBox.style.display = 'block';
setTimeout(() => { alertBox.style.display = 'none'; }, 5000);
}

// Signaling Error Handling
const socket = new WebSocket('ws://your-server-url');
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

new WebSocket('ws://your-server-url') is a non-functional placeholder and will break signaling in any real deployment. The existing codebase already derives the signaling URL from window.location.hostname (see client/main.js), so this should be wired into the real signaling URL/config rather than hardcoding a dummy string.

Suggested change
const socket = new WebSocket('ws://your-server-url');
const signalingProtocol = window.location.protocol === 'https:' ? 'wss://' : 'ws://';
const signalingHost = window.location.hostname;
const signalingPort = window.location.port ? `:${window.location.port}` : '';
const signalingUrl = `${signalingProtocol}${signalingHost}${signalingPort}`;
const socket = new WebSocket(signalingUrl);

Copilot uses AI. Check for mistakes.
socket.onerror = () => showAlert('Signaling server unreachable.');
socket.onclose = () => showAlert('Connection to server lost.');

// WebRTC Error Handling
const peerConnection = new RTCPeerConnection();
peerConnection.oniceconnectionstatechange = () => {
if (peerConnection.iceConnectionState === 'failed' || peerConnection.iceConnectionState === 'disconnected') {
showAlert('WebRTC connection failed.');
}
};

Comment on lines +16 to +22
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

Creating a standalone RTCPeerConnection() here doesn’t attach error/state listeners to the real peer connections used for transfers, so it won’t surface actual connection failures to users (and may create unnecessary resources). To implement the intended behavior, the ICE/connection state handling needs to be registered on the peer connection instances created/used by the app’s WebRTC flow (rather than a new, unused connection).

Suggested change
const peerConnection = new RTCPeerConnection();
peerConnection.oniceconnectionstatechange = () => {
if (peerConnection.iceConnectionState === 'failed' || peerConnection.iceConnectionState === 'disconnected') {
showAlert('WebRTC connection failed.');
}
};
function attachWebRTCErrorHandling(peerConnection) {
if (!peerConnection) {
return;
}
const handleICEConnectionStateChange = () => {
if (
peerConnection.iceConnectionState === 'failed' ||
peerConnection.iceConnectionState === 'disconnected'
) {
showAlert('WebRTC connection failed.');
}
};
peerConnection.addEventListener('iceconnectionstatechange', handleICEConnectionStateChange);
}

Copilot uses AI. Check for mistakes.
// File Transfer Error Handling
function handleTransfer(file) {
const reader = new FileReader();
reader.onerror = () => showAlert('File transfer interrupted.');
// ... transfer logic
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

handleTransfer() is never called, and the FileReader created inside it never starts a read operation (no readAs* call). This means the added reader.onerror handler will never actually fire for real transfers. The error handling should be hooked into the existing file-slice reading logic used during sending, and also consider data-channel close/error events to detect interrupted transfers.

Suggested change
// ... transfer logic
reader.onload = (event) => {
// Implement the actual transfer logic using event.target.result,
// for example by sending the data over a WebRTC data channel.
};
reader.readAsArrayBuffer(file);

Copilot uses AI. Check for mistakes.
}
109 changes: 14 additions & 95 deletions client/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,96 +1,15 @@
<!doctype html>
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100'><text y='.9em' font-size='90'>📡</text></svg>" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>OpenDrop - Secure Local WebRTC File Transfer</title>
<!-- Modern Font -->
<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Outfit:wght@300;400;600;700&display=swap" rel="stylesheet">
<!-- UI Icons -->
<link href="https://cdn.jsdelivr.net/npm/remixicon@4.2.0/fonts/remixicon.css" rel="stylesheet" />
<!-- Main CSS -->
<link rel="stylesheet" href="./style.css">
</head>
<body>
<!-- Hidden file input for selecting files -->
<input type="file" id="fileInput" style="display: none;" />
<input type="file" id="shareFileInput" style="display: none;" />

<header>
<div class="logo">
<i class="ri-radar-fill"></i> OpenDrop
</div>
<div class="header-actions">
<button class="share-link-btn" id="shareLinkBtn">
<i class="ri-link"></i> Share via Link
</button>
<div class="status">
<div class="status-indicator offline" id="connectionStatusIndicator"></div>
<span id="connectionStatusText">Connecting...</span>
</div>
</div>
</header>

<main>
<div class="radar-container">
<!-- Center profile (You) -->
<div class="my-profile">
<div class="avatar my-avatar">
<i class="ri-user-line"></i>
</div>
<div class="my-name-tag">
<span id="myName">Loading...</span>
<span class="label">You</span>
</div>
<!-- Radar waves -->
<div class="wave wave1"></div>
<div class="wave wave2"></div>
</div>

<!-- Peers will be dynamically injected here -->
<div id="peersContainer"></div>
</div>
</main>

<!-- Overlay UI for incoming files, progress, etc -->
<div id="toastContainer" class="toast-container"></div>

<div id="modalOverlay" class="modal-overlay hidden">
<div class="modal">
<h3 id="modalTitle">Incoming File</h3>
<div class="modal-content" id="modalContent">
<!-- File details injected dynamically -->
</div>
<div class="modal-actions" id="modalActions">
<!-- Action buttons injected dynamically -->
</div>
</div>
</div>

<script type="module" src="./main.js"></script>

<footer class="site-footer">
<p>Made by <strong>Dhanush Nehru</strong></p>
<div class="social-links">
<a href="https://x.com/Dhanush_Nehru" target="_blank" rel="noopener noreferrer" title="X (Twitter)">
<i class="ri-twitter-x-line"></i>
</a>
<a href="https://instagram.com/dhanush_nehru" target="_blank" rel="noopener noreferrer" title="Instagram">
<i class="ri-instagram-line"></i>
</a>
<a href="https://github.com/DhanushNehru" target="_blank" rel="noopener noreferrer" title="GitHub">
<i class="ri-github-fill"></i>
</a>
<a href="https://www.youtube.com/@dhanushnehru" target="_blank" rel="noopener noreferrer" title="YouTube">
<i class="ri-youtube-line"></i>
</a>
</div>
<a class="star-btn" href="https://github.com/DhanushNehru/OpenDrop" target="_blank" rel="noopener noreferrer">
<i class="ri-star-line"></i> Star on GitHub
</a>
</footer>
</body>
</html>
<head>
<meta charset="UTF-8">
<title>OpenDrop</title>
<style>
#alert-box { position: fixed; top: 20px; right: 20px; padding: 15px; border-radius: 5px; display: none; z-index: 1000; color: white; }
.error { background-color: #ff4d4d; }
</style>
</head>
<body>
<div id="alert-box"></div>
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

This change replaces the entire existing UI and removes all elements that the current frontend logic expects (e.g., connectionStatusIndicator, peersContainer, fileInput, modal/toast containers) while also no longer loading ./main.js as an ES module. As a result, the app’s peer discovery / transfer flow will not run at all. Instead of swapping the page to this minimal markup, keep the existing HTML/CSS structure and integrate the new user-facing error messaging into the current UI (e.g., via the existing toast container) while still loading ./main.js.

Suggested change
<div id="alert-box"></div>
<div id="app">
<header>
<h1>OpenDrop</h1>
<div id="connectionStatusIndicator"></div>
</header>
<main>
<section id="file-section">
<input id="fileInput" type="file" multiple>
</section>
<section id="peers-section">
<div id="peersContainer"></div>
</section>
</main>
<div id="modal-root"></div>
<div id="toast-container">
<div id="alert-box" class="error" style="display: none;"></div>
</div>
</div>
<script type="module" src="./main.js"></script>

Copilot uses AI. Check for mistakes.
<script src="app.js"></script>
</body>
Comment on lines +1 to +14
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

This diff is a large UI/asset rewrite (removes favicon/fonts/CSS, header/footer, and the whole interaction surface) rather than a targeted “basic error handling” change described in the PR. If the intent is only to add user-friendly alerts for signaling/WebRTC/file transfer errors, these UI deletions should be reverted and the error handling added to the existing implementation.

Copilot uses AI. Check for mistakes.
</html>