Fix data corruption

[corrideat]

Closes #108

[taoeffect]

Ran these changes through Opus and it may have found one bug:

Details

Bug: Missing backendTo.close() on Early Exit

The first shouldExit check exits without closing the destination backend, which could leave resources uncleaned or data unflushed. The second check handles this correctly, so they should be consistent.

    const value = await sbp('chelonia.db/get', `any:${key}`)
    if (shouldExit) {
      exit(shouldExit)
    }

Suggested fix:

    const value = await sbp('chelonia.db/get', `any:${key}`)
    if (shouldExit) {
      await backendTo.close()
      exit(shouldExit)
    }

---

Potential Issue: Type Safety with any: Prefix

The any: prefix will likely return a Buffer or Uint8Array for binary data. Ensure that backendTo.writeData(key, value) accepts this type. Without the type definition visible, it's worth verifying.

    const value = await sbp('chelonia.db/get', `any:${key}`)
    // ...
    await backendTo.writeData(key, value)

If writeData only accepts strings, you may need to update the DatabaseBackend interface or ensure all backends handle binary data appropriately.

---

Minor: Division by Zero Edge Case

If there are no keys in the database, numKeys would be 0:

    const percentage = Math.floor((numVisitedKeys / numKeys) * 100)

While the loop wouldn't execute in this case, explicitly guarding against this would be safer for future changes:

    const percentage = numKeys > 0 ? Math.floor((numVisitedKeys / numKeys) * 100) : 100

---

Question: Does any: Prefix Affect Key Storage?

When writing to the destination:

    await backendTo.writeData(key, value)

You're using the original key (without any: prefix) which appears correct. Just confirming this is intentional—the any: prefix is only for the read operation to get raw bytes, not stored as part of the key itself?

[taoeffect]

One last run:

Details

Bug: Typo in variable name

let interruputCount = 0

The variable name has a typo: "interruput" should be "interrupt".

Suggested fix:

let interruptCount = 0

And update the usage to match:

if (++interruptCount < 3) {

---

Potential Issue: Missing error handling around the migration loop

If backendTo.writeData(key, value) throws an error, backendTo.close() will never be called, potentially leaving resources open.

Suggested fix: Wrap the loop in a try-finally block:

let lastReportedPercentage = 0
try {
  for await (const key of sbp('chelonia.db/iterKeys')) {
    numVisitedKeys++
    if (!isValidKey(key)) {
      console.debug('[chel] Skipping invalid key', key)
      continue
    }
    // `any:` prefix needed to get the raw value, else the default is getting
    // a string, which will be encoded as UTF-8. This can cause data loss.
    const value = await sbp('chelonia.db/get', `any:${key}`)
    await checkAndExit()
    // Make `deno check` happy.
    if (value === undefined) {
      console.debug('[chel] Skipping empty key', key)
      continue
    }
    await backendTo.writeData(key, value)
    await checkAndExit()
    ++numMigratedKeys
    // Prints a message roughly every 10% of progress.
    const percentage = Math.floor((numVisitedKeys / numKeys) * 100)
    if (percentage - lastReportedPercentage >= 10) {
      lastReportedPercentage = percentage
      console.log(`[chel] Migrating... ${percentage}% done`)
    }
  }
  console.log(`[chel] ${colors.green('Migrated:')} ${numMigratedKeys} entries`)
} finally {
  await backendTo.close()
}

---

Minor: Signal exit code can be overwritten by subsequent signals

const handleSignal = (signal: string, code: number) => {
  process.on(signal, () => {
    // Exit codes follow the 128 + signal code convention.
    // See <https://tldp.org/LDP/abs/html/exitcodes.html>
    shouldExit = 128 + code

If a user sends SIGINT and then SIGTERM, shouldExit will change from 130 to 143. This is likely fine for a CLI tool, but if you want to preserve the original signal's exit code, consider:

if (!shouldExit) {
  shouldExit = 128 + code
}

---

Question: Does writeData handle Buffer/binary values correctly?

The fix retrieves the raw value with the any: prefix:

const value = await sbp('chelonia.db/get', `any:${key}`)
// ...
await backendTo.writeData(key, value)

This resolves the UTF-8 encoding corruption issue, but I wanted to confirm: does backendTo.writeData correctly handle whatever type any: returns (presumably Buffer or Uint8Array for binary data)? If this hasn't been tested across all backend implementations, it may be worth verifying.

[taoeffect]

Hopefully final run of Opus:

Details

Resource leak on error paths

When an error occurs during read or write operations, exit(1) is called without closing backendTo:

    } catch (e) {
      reportStatus()
      console.error('Error reading from source databse', key, e)
      exit(1)
    }

and

    } catch (e) {
      reportStatus()
      console.error('Error writing to target databse', key, e)
      exit(1)
    }

Suggested fix:

    } catch (e) {
      reportStatus()
      console.error('Error reading from source database', key, e)
      await backendTo.close()
      exit(1)
    }

    } catch (e) {
      reportStatus()
      console.error('Error writing to target database', key, e)
      await backendTo.close()
      exit(1)
    }

---

Division by zero when database is empty

If the source database has no keys (numKeys === 0), this will result in Infinity:

    const percentage = Math.floor((numVisitedKeys / numKeys) * 100)

Suggested fix:

    const percentage = numKeys > 0 ? Math.floor((numVisitedKeys / numKeys) * 100) : 100

Alternatively, add an early check at the start of the migration:

  const numKeys = await sbp('chelonia.db/keyCount')
  if (numKeys === 0) {
    console.log('[chel] Source database is empty. Nothing to migrate.')
    return
  }

[taoeffect]

Are any of these worth addressing in your mind or shall we consider this ready to merge?

Details

1. Type Annotation Mismatch

let value: Buffer | string

...later...

if (value === undefined) {

The type annotation says Buffer | string, but you're checking for undefined. This is inconsistent and could cause TypeScript confusion.

Suggested fix:

let value: Buffer | string | undefined

2. Force Quit Doesn't Close Database

} else {
  console.error(`Received signal ${signal} (${code}). Force quitting.`)
  reportStatus()
  exit(shouldExit)
}

When force quitting (after 3 interrupts), backendTo.close() is never called. This could leave the destination database in an inconsistent state.

Suggested fix:

} else {
  console.error(`Received signal ${signal} (${code}). Force quitting.`)
  reportStatus()
  backendTo.close().finally(() => exit(shouldExit))
}

Or if you want synchronous behavior in the signal handler:

} else {
  console.error(`Received signal ${signal} (${code}). Force quitting.`)
  reportStatus()
  // Best-effort close; don't await in signal handler
  void backendTo.close()
  exit(shouldExit)
}

3. Potential Use of Uninitialized Variable

If exit(1) doesn't terminate execution (e.g., if mocked in tests or if it throws internally), the code could continue with value uninitialized:

try {
  value = await sbp('chelonia.db/get', `any:${key}`)
} catch (e) {
  reportStatus()
  console.error(`Error reading from source database key '${key}'`, e)
  await backendTo.close()
  exit(1)
}
await checkAndExit()
// value could be uninitialized here if exit() doesn't terminate
if (value === undefined) {

Suggested fix — use throw or return after exit() to satisfy control flow analysis:

} catch (e) {
  reportStatus()
  console.error(`Error reading from source database key '${key}'`, e)
  await backendTo.close()
  exit(1)
  throw e // Satisfy control flow; never reached if exit() terminates
}

[taoeffect]

Great work!!