Audit: Replace panics with Result returns #132

New issue

Closed

opened 2025-12-29 08:29:19 +00:00 by navicore · 1 comment

navicore commented

2025-12-29 08:29:19 +00:00

(Migrated from github.com)

Summary

Audit all builtins and runtime functions to replace panics with proper Result/error returns. Errors should be values, not crashes.

Background

During the design discussion for Weaves (#131), we identified that several existing builtins use Rust panics for error conditions that should be recoverable:

"Building in a guaranteed crash of the process is hard to take seriously. The only reason to panic ever in a program is that you need some external help to recover."

Known Issues

Channel operations

chan.receive - panics on closed channel
chan.send - panics on closed channel
The -safe variants exist (chan.send-safe, chan.receive-safe) but the naming is backwards

Naming convention problem

The current -safe suffix implies the default is unsafe. This is backwards:

Current: chan.receive (panics) vs chan.receive-safe (returns Result)
Better: chan.receive (returns Result) vs chan.receive! or chan.receive-unchecked (panics for perf-critical code)

Proposed Changes

Audit all builtins in builtins.rs and their runtime implementations
Convert panics to Results for all user-facing error conditions
Deprecate -safe suffix - make safe the default
Add -unchecked! suffix for explicit "I know what I'm doing" variants that skip checks for performance

Result Pattern

Seq already has std:result with proper Result/Option handling:

include std:result

# Functions should return Results
: safe-divide ( Int Int -- IntResult )
  dup 0 i.= if
    drop "Division by zero" Make-Err
  else
    i./ Make-Ok
  then
;

# Callers handle errors explicitly
10 0 safe-divide
result-ok? if
  result-unwrap use-value
else
  result-unwrap-err log-error
then

Audit Checklist

chan.send / chan.receive
variant.field-at (out of bounds)
list.map / list.filter / list.fold (quotation errors)
File operations
String operations (index out of bounds, etc.)
Type conversion operations
Any other runtime panics

Non-Goals

Some panics are appropriate and should remain:

Internal invariant violations (bugs in the runtime)
Stack corruption / null pointer dereference
Truly unrecoverable states

The distinction: user errors (bad input, closed channel, file not found) should return Results. Runtime bugs (impossible states, invariant violations) can panic.

## Summary Audit all builtins and runtime functions to replace panics with proper Result/error returns. Errors should be values, not crashes. ## Background During the design discussion for [Weaves (#131)](https://github.com/navicore/patch-seq/issues/131), we identified that several existing builtins use Rust panics for error conditions that should be recoverable: > "Building in a guaranteed crash of the process is hard to take seriously. The only reason to panic ever in a program is that you need some external help to recover." ## Known Issues ### Channel operations - `chan.receive` - panics on closed channel - `chan.send` - panics on closed channel - The `-safe` variants exist (`chan.send-safe`, `chan.receive-safe`) but the naming is backwards ### Naming convention problem The current `-safe` suffix implies the default is unsafe. This is backwards: - **Current:** `chan.receive` (panics) vs `chan.receive-safe` (returns Result) - **Better:** `chan.receive` (returns Result) vs `chan.receive!` or `chan.receive-unchecked` (panics for perf-critical code) ## Proposed Changes 1. **Audit all builtins** in `builtins.rs` and their runtime implementations 2. **Convert panics to Results** for all user-facing error conditions 3. **Deprecate `-safe` suffix** - make safe the default 4. **Add `-unchecked!` suffix** for explicit "I know what I'm doing" variants that skip checks for performance ## Result Pattern Seq already has `std:result` with proper Result/Option handling: ```seq include std:result # Functions should return Results : safe-divide ( Int Int -- IntResult ) dup 0 i.= if drop "Division by zero" Make-Err else i./ Make-Ok then ; # Callers handle errors explicitly 10 0 safe-divide result-ok? if result-unwrap use-value else result-unwrap-err log-error then ``` ## Audit Checklist - [ ] `chan.send` / `chan.receive` - [ ] `variant.field-at` (out of bounds) - [ ] `list.map` / `list.filter` / `list.fold` (quotation errors) - [ ] File operations - [ ] String operations (index out of bounds, etc.) - [ ] Type conversion operations - [ ] Any other runtime panics ## Non-Goals Some panics are appropriate and should remain: - Internal invariant violations (bugs in the runtime) - Stack corruption / null pointer dereference - Truly unrecoverable states The distinction: **user errors** (bad input, closed channel, file not found) should return Results. **Runtime bugs** (impossible states, invariant violations) can panic.

navicore commented

2025-12-30 02:29:12 +00:00

(Migrated from github.com)

final fix https://github.com/navicore/patch-seq/pull/137