Skip to content

Improve Error Handling and Recursion Prevention in Type Resolution #3540

Description

@jasonbahl

Improve Error Handling and Recursion Prevention in Type Resolution

Context

While working on the fix for PR #3539 (fix: Allow compatible interface field override with register_graphql_field()), I encountered recursion and error handling challenges that led me to implement a solution using:

  • Class properties for recursion guards
  • Try/catch blocks for graceful error handling during type resolution

This pattern proved effective and made me realize we could apply similar improvements throughout the codebase to enhance robustness and developer experience.

The Problem

During type resolution, especially when dealing with:

  • Interface field overrides
  • Lazy-loaded types (callables)
  • Recursive type unwrapping
  • Eager type loading

We currently have several areas where:

  1. Errors bubble up unhandled - breaking schema building instead of gracefully degrading
  2. No recursion guards - risking infinite loops or stack overflows
  3. Poor error messages - making debugging difficult when things go wrong

Proposed Improvements

I've audited the codebase and identified 6 key areas for improvement:

  1. TypeRegistry::get_type() - Type loader closures can throw, breaking schema building
  2. TypeRegistry::prepare_field() - Type resolution in closures lacks error handling
  3. TypeRegistry::setup_type_modifiers() - Recursive function with no depth limit, risking stack overflow
  4. TypeRegistry::get_eager_type_map() - One bad type breaks all eager loading
  5. WPInterfaceTrait::resolve_inherited_interfaces() - Interface resolution could throw
  6. Array type modifier closure - Add try/catch for consistency

Benefits

Implementing these improvements will:

Prevent schema building failures - Bad type configurations won't break the entire schema
Improve developer experience - Better error messages via graphql_debug()
Prevent stack overflows - Depth limits on recursive functions
Graceful degradation - One bad type doesn't break everything
Consistent patterns - Establish best practices for error handling in type resolution

Implementation Approach

The pattern we used in PR #3539 works well:

// Recursion guard with class property
protected $checking_compatibility = false;

if ( ! $this->checking_compatibility ) {
    $this->checking_compatibility = true;
    
    try {
        // Type resolution logic
    } catch ( \Throwable $e ) {
        // Graceful error handling with graphql_debug()
    } finally {
        $this->checking_compatibility = false;
    }
}

This can be adapted for other scenarios:

  • Class methods: Use class properties for recursion guards
  • Recursive functions: Add depth limits
  • Type loading: Wrap in try/catch with helpful error messages

Detailed Improvements

1. TypeRegistry::get_type() - Type Loader Error Handling

Location: src/Registry/TypeRegistry.php:1008-1024

Problem: When a type loader closure throws an exception, it bubbles up and can break the entire schema building process. A single malformed type configuration can prevent the schema from being built at all.

Solution: Wrap the type loader execution in a try/catch block. When an error occurs, log it via graphql_debug() with helpful context (type name, error message), mark the type as failed to prevent retry loops, and allow schema building to continue. This ensures graceful degradation - one bad type won't break everything.

Approach:

// Pseudo-code pattern
try {
    $type = $this->type_loaders[ $key ]();
    // ... store type
} catch ( \Throwable $e ) {
    graphql_debug( /* helpful error message */ );
    // Mark as failed, prevent retry
}

2. TypeRegistry::prepare_field() - Type Resolution in Closure

Location: src/Registry/TypeRegistry.php:1115-1130

Problem: The closure that resolves field types calls get_type(), which could throw if the type loader fails (see #1). Currently, these exceptions aren't caught, leading to unhelpful error messages or schema building failures.

Solution: Wrap the get_type() call in a try/catch block. If an exception occurs during type resolution, catch it and throw a more helpful error message that includes the field name and type context, making it easier for developers to debug configuration issues.


3. TypeRegistry::setup_type_modifiers() - Recursive Type Unwrapping

Location: src/Registry/TypeRegistry.php:1189-1207

Problem: This recursive function unwraps type modifiers (like non_null, list_of) but has no depth limit. Malformed type arrays could cause infinite recursion and stack overflow. Additionally, the non_null() and list_of() methods could throw if given invalid types.

Solution:

  • Add a depth parameter with a maximum limit (e.g., 10 levels) to prevent stack overflow
  • Wrap modifier application in try/catch blocks
  • When depth is exceeded or an error occurs, log via graphql_debug() and return a safe fallback type (e.g., Type::string())

Approach:

// Pseudo-code pattern
function setup_type_modifiers( $type, int $depth = 0 ) {
    if ( $depth > MAX_DEPTH ) {
        // Log error, return fallback
    }
    try {
        // Recursive call with depth + 1
    } catch ( \Throwable $e ) {
        // Log error, return fallback
    }
}

4. TypeRegistry::get_eager_type_map() - Eager Type Loading

Location: src/Registry/TypeRegistry.php:239-250

Problem: When eagerly loading types, if get_type() throws for one type, it breaks the entire eager loading process. This means one bad type prevents all eager types from being loaded.

Solution: Wrap each get_type() call in a try/catch block within the loop. If one type fails to load, log the error via graphql_debug() but continue loading the remaining types. This ensures that one problematic type doesn't prevent other types from being eagerly loaded.


5. WPInterfaceTrait::resolve_inherited_interfaces() - Recursive Interface Resolution

Location: src/Type/WPInterfaceTrait.php:135-160

Problem: While the method uses array keys to prevent duplicate interfaces (which provides some recursion protection), the getInterfaces() call could throw an exception. If this happens, the entire interface resolution process fails.

Solution: Wrap the getInterfaces() call in a try/catch block. If an exception occurs, log it via graphql_debug() and return early, allowing the rest of the schema building to continue. The existing array key protection already handles recursion, so we just need to add error handling.


6. TypeRegistry::prepare_field() - Array Type Modifier Closure

Location: src/Registry/TypeRegistry.php:1145-1147

Problem: The closure that sets up type modifiers for array-based type configurations doesn't have error handling. If setup_type_modifiers() throws (see #3), it will bubble up unhandled.

Solution: Wrap the setup_type_modifiers() call in a try/catch block. If an error occurs, log it via graphql_debug() and return a safe fallback type. This ensures consistency with other type resolution closures and prevents unhandled exceptions.

Testing Strategy

For each improvement:

  1. Add tests that trigger the error condition
  2. Verify graceful degradation (schema still builds)
  3. Verify helpful error messages via graphql_debug()
  4. Ensure no performance regression

Next Steps

  • Review and discuss the proposed improvements
  • Implement all improvements together in a single PR (or group logically)
  • Add tests for each improvement
  • Document the patterns in coding standards

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    Status
    ✅ Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions