From 7793163e0b687709f556a8e3fa41473878d3a5da Mon Sep 17 00:00:00 2001
From: tiennm99 <tiennm99@outlook.com>
Date: Thu, 16 Apr 2026 16:37:07 +0700
Subject: [PATCH] fix: critical Upstash API and UX issues from second review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix setPixels() to use Upstash bitfield builder (.set().exec())
  instead of flat array (would throw at runtime)
- Fix getFullCanvas() to use GETRANGE for reliable binary retrieval
  instead of GET which may mangle BITFIELD data
- Fix getUserId hash overflow: use >>> 0 instead of Math.abs()
- Add client-side credit regeneration timer (1/sec)
- Add exponential backoff to WebSocket reconnection (1s → 30s cap)
---
 src/client/App.svelte     | 22 +++++++++++++++++++---
 src/lib/canvas-storage.js | 34 +++++++++++++++++++++++-----------
 src/lib/get-user-id.js    |  2 +-
 3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/src/client/App.svelte b/src/client/App.svelte
index 47a456c..4bc19b7 100644
--- a/src/client/App.svelte
+++ b/src/client/App.svelte
@@ -1,5 +1,5 @@
 <script>
-  import { MAX_CREDITS } from '../lib/constants.js';
+  import { MAX_CREDITS, CREDIT_REGEN_RATE } from '../lib/constants.js';
   import CanvasRenderer from './components/CanvasRenderer.svelte';
   import ColorPicker from './components/ColorPicker.svelte';
   import CanvasControls from './components/CanvasControls.svelte';
@@ -13,7 +13,19 @@
   /** @type {CanvasRenderer} */
   let canvasRenderer;
 
-  // WebSocket connection with auto-reconnect
+  // Client-side credit regeneration (server corrects on placement)
+  $effect(() => {
+    const interval = setInterval(() => {
+      if (credits < MAX_CREDITS) {
+        credits = Math.min(credits + CREDIT_REGEN_RATE, MAX_CREDITS);
+      }
+    }, 1000);
+    return () => clearInterval(interval);
+  });
+
+  // WebSocket connection with auto-reconnect + exponential backoff
+  let wsRetryDelay = 1000;
+
   function connectWebSocket() {
     const protocol = location.protocol === 'https:' ? 'wss:' : 'ws:';
     const ws = new WebSocket(`${protocol}//${location.host}/api/ws`);
@@ -27,7 +39,11 @@
       } catch { /* ignore parse errors */ }
     };
 
-    ws.onclose = () => setTimeout(connectWebSocket, 1000);
+    ws.onopen = () => { wsRetryDelay = 1000; };
+    ws.onclose = () => {
+      setTimeout(connectWebSocket, wsRetryDelay);
+      wsRetryDelay = Math.min(wsRetryDelay * 2, 30000);
+    };
     ws.onerror = () => ws.close();
 
     return ws;
diff --git a/src/lib/canvas-storage.js b/src/lib/canvas-storage.js
index 9e21c32..475f54c 100644
--- a/src/lib/canvas-storage.js
+++ b/src/lib/canvas-storage.js
@@ -6,29 +6,42 @@ const CANVAS_BYTES = Math.ceil((TOTAL_PIXELS * BITS_PER_PIXEL) / 8);
 
 /**
  * Get the full canvas as a Uint8Array of raw bytes.
+ * Uses GETRANGE to fetch raw binary reliably (avoids encoding issues with GET).
  * Returns a zero-filled buffer if canvas doesn't exist yet.
  * @param {object} env
  * @returns {Promise<Uint8Array>}
  */
 export async function getFullCanvas(env) {
   const redis = getRedis(env);
-  const data = await redis.get(REDIS_CANVAS_KEY);
-  if (!data) {
+
+  // Use GETRANGE to fetch the full string as raw bytes
+  // This is more reliable than GET for binary data written by BITFIELD
+  const data = await redis.getrange(REDIS_CANVAS_KEY, 0, CANVAS_BYTES - 1);
+  if (!data || data.length === 0) {
     return new Uint8Array(CANVAS_BYTES);
   }
+
+  // Upstash REST returns string — convert to bytes
   if (typeof data === 'string') {
-    const binary = atob(data);
-    const bytes = new Uint8Array(binary.length);
-    for (let i = 0; i < binary.length; i++) {
-      bytes[i] = binary.charCodeAt(i);
+    const bytes = new Uint8Array(data.length);
+    for (let i = 0; i < data.length; i++) {
+      bytes[i] = data.charCodeAt(i);
+    }
+    // Pad to full canvas size if shorter
+    if (bytes.length < CANVAS_BYTES) {
+      const padded = new Uint8Array(CANVAS_BYTES);
+      padded.set(bytes);
+      return padded;
     }
     return bytes;
   }
-  return new Uint8Array(data);
+
+  return new Uint8Array(CANVAS_BYTES);
 }
 
 /**
  * Set multiple pixels in a single atomic BITFIELD command.
+ * Uses Upstash's builder pattern: redis.bitfield(key).set().set().exec()
  * @param {object} env
  * @param {Array<{x: number, y: number, color: number}>} pixels
  */
@@ -36,11 +49,10 @@ export async function setPixels(env, pixels) {
   if (!pixels.length) return;
   const redis = getRedis(env);
 
-  const commands = [];
+  let chain = redis.bitfield(REDIS_CANVAS_KEY);
   for (const { x, y, color } of pixels) {
     const offset = y * CANVAS_WIDTH + x;
-    commands.push('SET', 'u5', `#${offset}`, color);
+    chain = chain.set('u5', `#${offset}`, color);
   }
-
-  await redis.bitfield(REDIS_CANVAS_KEY, commands);
+  await chain.exec();
 }
diff --git a/src/lib/get-user-id.js b/src/lib/get-user-id.js
index cec8fc2..e7c8091 100644
--- a/src/lib/get-user-id.js
+++ b/src/lib/get-user-id.js
@@ -13,5 +13,5 @@ export function getUserId(request) {
   for (let i = 0; i < ip.length; i++) {
     hash = ((hash << 5) - hash + ip.charCodeAt(i)) | 0;
   }
-  return `anon:${Math.abs(hash).toString(36)}`;
+  return `anon:${(hash >>> 0).toString(36)}`;
 }