Skip to content

Conversation

@chrisli30
Copy link
Member

@chrisli30 chrisli30 commented Oct 7, 2025

This pull request refactors the node output processing logic in core/taskengine/run_node_immediately.go to use a handler-based, object-oriented approach. The main goal is to eliminate code duplication, centralize output handling for different node types, and improve maintainability. Instead of using large switch statements, the code now delegates output extraction and protobuf conversion to specialized handlers.

Refactoring for handler-based node output processing:

  • Replaced the large switch statement in extractExecutionResult with a handler-based approach using a NodeOutputHandlerFactory and node-specific handlers, which centralizes and simplifies output extraction for all node types.
  • Added a new helper method detectNodeTypeFromStep to determine the node type from the execution step as a fallback when the type is not explicitly set.

Improvements to RPC response construction:

  • Refactored error handling in RunNodeImmediatelyRPC to use handlers for creating empty output structures on failure, replacing the previous switch-based logic.
  • Updated successful RPC response construction to use handlers for converting results to protobuf output, including proper metadata handling and special logic for contract write success determination.

Codebase cleanup:

  • Removed legacy switch statements and redundant metadata extraction logic, consolidating all output and metadata handling into the new handler-based pattern.

Dependency management:

  • Cleaned up unused imports related to legacy output processing utilities.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the execution output logic by introducing an object-oriented approach that replaces the large switch statements with a factory pattern and individual handler classes for each node type. This improves code maintainability and eliminates duplication in output processing logic.

Key changes:

  • Introduces a NodeOutputHandler interface and factory pattern for managing different node types
  • Replaces extensive switch statements with handler-based delegation in extractExecutionResult and RunNodeImmediatelyRPC
  • Creates individual handler classes for each node type (RestAPI, CustomCode, Balance, etc.) in the new file

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
core/taskengine/run_node_immediately.go Refactored to use handler-based approach, removed large switch statements, added node type detection fallback
core/taskengine/node_output_handlers.go New file containing handler interface, factory, and individual handlers for each node type

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@chrisli30 chrisli30 merged commit 142fd84 into staging Oct 7, 2025
17 checks passed
@chrisli30 chrisli30 deleted the chris-refactor_run_node branch October 7, 2025 06:40
chrisli30 added a commit that referenced this pull request Oct 17, 2025
…ch (#412)

* fix: refactor the execution output logic using object-oriented approach

* added helper functin for type conversion: assignOutputData

* fix: metadata field not available after refactor

* Remove the deprecated old code

* fix: added two-phase balance retrival for BalanceNode

* Added integration tests for the BalanceNode

* fix: test of  TestBalanceNode_MissingAPIKey

---------

Co-authored-by: Chris Li <[email protected]>
chrisli30 added a commit that referenced this pull request Oct 17, 2025
…ch (#412)

* fix: refactor the execution output logic using object-oriented approach

* added helper functin for type conversion: assignOutputData

* fix: metadata field not available after refactor

* Remove the deprecated old code

* fix: added two-phase balance retrival for BalanceNode

* Added integration tests for the BalanceNode

* fix: test of  TestBalanceNode_MissingAPIKey

---------

Co-authored-by: Chris Li <[email protected]>
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