Skip to content

Conversation

@dcrockwell
Copy link
Contributor

Summary

This PR fixes critical bugs in dream_ets that made persistence operations completely non-functional, improves error handling to prevent panics, and adds comprehensive documentation with verified examples.

Why These Changes Were Made

While working on documentation, we discovered that example code snippets couldn't be verified because they weren't being compiled. This led to discovering several critical issues:

  1. Persistence operations were completely broken - save_to_file() and load_from_file() had never been tested and didn't work due to FFI bugs
  2. Error handling violated our standards - Code was throwing away errors with Error(_) patterns and panicking on decode failures
  3. Documentation taught bad practices - Examples showed Error(_) patterns users would copy into production
  4. Test coverage gaps - Several public functions had zero tests, allowing bugs to ship

The root cause: untested code is broken code. The persistence operations had zero unit tests and were fundamentally broken.

What Was Changed

Critical Bug Fixes

FFI Bugs Fixed:

  • save_to_file() / load_from_file(): Fixed binary/charlist conversion - Gleam Strings are binaries but Erlang file operations need charlists
  • update_element(): Fixed return type from ok atom to {ok, nil} tuple
  • ets_first(): Fixed return value from empty to empty_table to match type system

Error Handling Fixed:

  • Removed all Error(_) patterns - now explicitly handle all error variants
  • Removed panic calls from keys(), values(), to_list() helper functions
  • Added proper error propagation with descriptive messages

Breaking API Changes (v1.0.2 → v2.0.0)

Four functions now return Result instead of bare values to properly handle decode errors:

  • operations.keys(): List(k)Result(List(k), EtsError)
  • operations.values(): List(v)Result(List(v), EtsError)
  • operations.to_list(): List(#(k,v))Result(List(#(k,v)), EtsError)
  • operations.size(): IntResult(Int, EtsError)

Why this is necessary: These functions iterate the table and decode keys/values. If decode fails (corrupt data, encoder mismatch, migration issues), the old code would panic and crash the user's process. Now users can handle decode errors gracefully.

Migration is simple:

// Before (1.x):
let all_keys = operations.keys(table)

// After (2.x):
case operations.keys(table) {
  Ok(all_keys) -> use_keys(all_keys)
  Error(err) -> handle_corruption(err)
}

Documentation & Testing

Test Coverage:

  • Added 15 new tests across 4 new test suites
  • Increased from 62 to 77 tests (24% increase)
  • Achieved 100% coverage of public API in src/dream_ets/
  • Previously untested functions now have tests:
    • Persistence operations (save/load)
    • Advanced operations (update_element, match, select)
    • Encoders (bool, float)
    • Config options (keypos, bag types)

Documentation:

  • World-class hexdocs for all 100+ public functions and types
  • 9 tested code snippets showing realistic use cases
  • Completely rewrote README with proper error handling examples
  • Added custom type encoding example (JSON)
  • Added preventing duplicates example (insert_new)

Impact Assessment

Who's Affected:

  • Anyone using keys(), values(), to_list(), or size() needs to handle Result
  • Core Dream and examples are NOT affected (verified - all tests pass)

Who Benefits:

  • Everyone using persistence operations (were broken, now work)
  • Anyone with custom encoders (can now handle decode errors instead of crashes)
  • New users learning from documentation (now see correct patterns)

Testing

✅ dream_ets: 77 tests, 0 failures
✅ Dream core: 239 tests, 0 failures
✅ rate_limiter example: 8 integration tests, 0 failures
✅ streaming example: 6 integration tests, 0 failures

Total: 330 tests across entire codebase, all passing

Files Changed

  • 57 files changed (+4000 lines, -1417 lines)
  • Core modules: config.gleam, operations.gleam, helpers.gleam, encoders.gleam
  • FFI: internal_ffi.erl (charlist conversions, return type fixes)
  • Tests: 4 new test files, 15 new tests
  • Examples: 9 new verified snippets in test/snippets/
  • Docs: README.md, CHANGELOG.md, comprehensive hexdocs

Review Notes

Focus areas for review:

  1. FFI changes (charlist conversion, return types) - critical bug fixes
  2. Breaking API changes (Result returns) - verify this is the right approach
  3. Error handling improvements (no more panics/Error(_)) - verify standards compliance

This is a significant quality improvement that fixes production-breaking bugs while maintaining backward compatibility for the most common use cases.

…rehensive documentation

## Why This Change Was Made

- Persistence operations (save_to_file/load_from_file) were COMPLETELY BROKEN due to binary/charlist mismatch in FFI
- Error handling violated coding standards by throwing away errors with Error(_) patterns and panicking
- Documentation examples taught users BAD patterns (Error(_) everywhere)
- Test coverage gaps meant bugs went undetected (persistence had ZERO unit tests)
- Functions claimed to never fail (keys() -> List) but could panic on decode errors

This was discovered when trying to verify documentation snippets actually worked.
The type error comment in the snippet couldn't be verified because snippets weren't compiled.
Following the chain led to: untested snippets → broken examples → missing tests → FFI bugs.

## What Was Changed

### Critical Bug Fixes (FFI)
- save_to_file: Convert Gleam String (binary) to Erlang charlist for ets:tab2file
- load_from_file: Same charlist conversion for ets:file2tab
- update_element: Return {ok, nil} instead of ok atom
- ets_tab2file/ets_file2tab: Return {ok, nil} instead of ok atom
- ets_first: Return empty_table instead of empty atom

### Breaking API Changes (Proper Error Handling)
- keys() now returns Result(List(k), EtsError) - can fail on decode errors
- values() now returns Result(List(v), EtsError) - can fail on decode errors
- to_list() now returns Result(List(#(k,v)), EtsError) - can fail on decode errors
- size() now returns Result(Int, EtsError) - depends on keys() which can fail

Removed all panic calls - libraries shouldn't crash user processes. Added proper error
propagation with helpful messages (e.g., 'Key disappeared during iteration').

### Documentation & Testing
- Added world-class hexdocs for all 100+ public functions and types
- Added 4 new test suites: persistence_test, encoders_test, helpers_test, advanced_operations_test
- Increased test coverage from 62 to 77 tests (100% of public API)
- Created 9 verified, tested code snippets (moved to test/snippets/)
- Completely rewrote README with actual tested code (no more Error(_) anti-patterns)
- Fixed all Error(_) and Error(_error) violations in source code

### New Examples
- Custom type encoding with JSON (critical for real apps)
- Preventing duplicates with insert_new() (distributed locks, user registration)

## Note to Future Engineer

The charlist bug is a classic Gleam/Erlang interop gotcha - Gleam Strings are binaries,
but many Erlang file operations expect charlists. Always check Erlang docs for string types.

The panic-on-decode-error bug was insidious because it only happens when:
1. You change your encoder/decoder (migration)
2. Data gets corrupted
3. You have a bug in custom encoder

Without Result types, users had no way to handle this gracefully. Their app would just crash
when iterating keys. Now they can catch decode errors and recover (delete corrupt entries,
log issues, etc.).

Oh, and the persistence operations had ZERO tests before this. They were completely broken
and nobody knew because... nobody tested them. Remember: if it's not tested, it's broken.
You just don't know it yet. 💀

BREAKING CHANGE: keys(), values(), to_list(), and size() now return Result types instead of bare values
@dcrockwell dcrockwell merged commit 516318c into develop Dec 8, 2025
2 checks passed
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