Skip to content

feat: Add error recovery system#450

Open
rlch wants to merge 2 commits intoalecthomas:masterfrom
rlch:feat/error-recovery
Open

feat: Add error recovery system#450
rlch wants to merge 2 commits intoalecthomas:masterfrom
rlch:feat/error-recovery

Conversation

@rlch
Copy link
Copy Markdown

@rlch rlch commented Nov 26, 2025

Implements error recovery inspired by Chumsky's recovery system in Rust, addressing GitHub issue #342.

Features:

  • RecoveryStrategy interface with multiple implementations:
    • SkipUntil/SkipPast: classic panic-mode recovery
    • SkipThenRetryUntil: skip and retry parsing
    • NestedDelimiters: balanced delimiter recovery
    • TokenSyncStrategy: sync on token types
    • CompositeStrategy: try multiple strategies in order
  • RecoveryError type for multi-error reporting
  • ParseOptions: Recover(strategies...) and MaxRecoveryErrors(n)

The implementation allows parsers to continue after errors, collecting
multiple errors and producing partial ASTs - useful for IDE integration,
linters, and comprehensive error messages.

Includes comprehensive test coverage (100% on new code) and example at
_examples/recovery/.

rlch and others added 2 commits November 26, 2025 20:26
Implements error recovery inspired by Chumsky's recovery system in Rust,
addressing GitHub issue alecthomas#342.

New features:
- RecoveryStrategy interface with multiple implementations:
  - SkipUntil/SkipPast: classic panic-mode recovery
  - SkipThenRetryUntil: skip and retry parsing
  - NestedDelimiters: balanced delimiter recovery
  - TokenSyncStrategy: sync on token types
  - CompositeStrategy: try multiple strategies in order
- RecoveryError type for multi-error reporting
- ParseOptions: Recover(strategies...) and MaxRecoveryErrors(n)

Integration points:
- parseContext with tryRecover(), recoveryEnabled(), addRecoveryError()
- Recovery hooks in strct.Parse and group.Parse (nodes.go)
- Error aggregation in parser.go

The implementation allows parsers to continue after errors, collecting
multiple errors and producing partial ASTs - useful for IDE integration,
linters, and comprehensive error messages.

Includes comprehensive test coverage (100% on new code) and example at
_examples/recovery/.

Amp-Thread-ID: https://ampcode.com/threads/T-b95d5ac9-6023-4947-a477-53364956f203
Co-authored-by: Amp <amp@ampcode.com>
Extends the error recovery system with Chumsky-inspired features:

Per-Node Recovery (via struct tags):
- Add 'recover' struct tag for per-field recovery strategies
- Syntax: \`parser:"@Ident" recover:"skip_until(;)"\`
- Supports: skip_until(), skip_past(), nested(), retry_until()
- Multiple strategies via pipe: \`recover:"nested((,))|skip_until(;)"\`
- Labels for error messages: \`recover:"label:expression|skip_until(;)"\`

Recovery Metadata Fields:
- Recovered bool - set to true when struct was recovered
- RecoveredSpan lexer.Position - position where recovery started
- Auto-injected like Pos/EndPos fields

Implementation:
- recoveryNode wrapper applies recovery to any node
- Handles both errors AND 'no match' cases (nil, nil)
- Creates meaningful error messages for recovery reporting
- Updated visit.go, ebnf.go for recoveryNode support

This brings participle closer to Chumsky's recovery capabilities,
enabling fine-grained recovery control per grammar rule.

Amp-Thread-ID: https://ampcode.com/threads/T-b95d5ac9-6023-4947-a477-53364956f203
Co-authored-by: Amp <amp@ampcode.com>
Copy link
Copy Markdown
Owner

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

This is awesome! I'll try to take a comprehensive look on the weekend.

Copy link
Copy Markdown
Owner

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

Apologies for the delay, this is a great idea!

I've left a few comments, but I'll do a full pass in a bit.

Comment thread recovery.go
Comment on lines +421 to +429
if strings.HasPrefix(tag, "label:") {
parts := strings.SplitN(tag[6:], "|", 2)
config.label = strings.TrimSpace(parts[0])
if len(parts) > 1 {
tag = parts[1]
} else {
return config, nil
}
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This seems redundant given that the strategy loop also checks for the label?

Comment thread recovery.go

// recoveryNode wraps another node with recovery configuration.
// This allows any node to have per-node recovery without modifying all node types.
type recoveryNode struct {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There are a few locations where the code type switches on the node type that will need to be updated. You're probably going to have to make this public, and add a method that "unwraps" the underlying node for introspection.

Comment thread recovery.go
Comment on lines +600 to +602
// =============================================================================
// Field Recovery Tag Extraction
// =============================================================================
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you get rid of these section markers please?

@alecthomas
Copy link
Copy Markdown
Owner

Apologies for the delay BTW, I've been absolutely swamped at work.

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