Title: Fatal error when ToolBase::getResult() is called before result is initialised

Note: This issue was created with ChatGPT, and I just couldn't get it to do what I wanted with this issue, so apologies for the mess. There is the error message and the attempt to explain it below.

Problem/Motivation

The Tool module can throw a fatal PHP error when
Drupal\tool\Tool\ToolBase::getResult() is called before the typed
$result property has been initialised.

This occurs in integrations that expect a tool to expose readable output. For example, when tools are invoked via tool_ai_connector. If the tool does not successfully execute, or exits early, the result may never be set.

Because $result is a typed property, accessing it before initialisation causes a hard PHP error.

Steps to reproduce

  1. Create or configure a tool that extends ToolBase.
  2. Ensure the tool does not always set a result.
  3. Invoke the tool via an integration that calls getResult() or getReadableOutput().

Observed error:

 Error: Typed property Drupal\tool\Tool\ToolBase::$result must not be accessed before initialization

Proposed resolution

Ensure ToolBase can be safely consumed when no result is produced.

One option is to initialise $result in the base class.

Another option is to guard getResult() against an uninitialised state.

For example, return null or throw a controlled exception instead of triggering a fatal error.

Tool implementations should have a clear contract for when a result must be set.

Error Output

Error: Typed property Drupal\tool\Tool\ToolBase::$result must not be accessed before initialization in Drupal\tool\Tool\ToolBase->getResult() (line 157 of /var/www/html/web/modules/contrib/tool/src/Tool/ToolBase.php).

#0 /var/www/html/web/modules/contrib/tool/modules/tool_ai_connector/src/Plugin/AiFunctionCall/ToolPluginBase.php(131): Drupal\tool\Tool\ToolBase->getResult()
#1 /var/www/html/web/modules/contrib/ai_agents/src/PluginBase/AiAgentEntityWrapper.php(1308): Drupal\tool_ai_connector\Plugin\AiFunctionCall\ToolPluginBase->getReadableOutput()
#2 /var/www/html/web/modules/contrib/ai/modules/ai_assistant_api/src/Service/AgentRunner.php(91): Drupal\ai_agents\PluginBase\AiAgentEntityWrapper->toArray()
#3 /var/www/html/web/modules/contrib/ai/modules/ai_assistant_api/src/AiAssistantApiRunner.php(319): Drupal\ai_assistant_api\Service\AgentRunner->runAsAgent()
#4 /var/www/html/web/modules/contrib/ai/modules/ai_chatbot/src/Controller/DeepChatApi.php(203): Drupal\ai_assistant_api\AiAssistantApiRunner->process()
#5 [internal function]: Drupal\ai_chatbot\Controller\DeepChatApi->api()
#6 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array()
#7 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(634): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#8 [internal function]: Drupal\Core\Render\Renderer::Drupal\Core\Render\{closure}()
#9 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(649): Fiber->resume()
#10 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(121): Drupal\Core\Render\Renderer->executeInRenderContext()
#11 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext()
#12 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(183): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#13 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#14 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle()
#15 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle()
#16 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#17 /var/www/html/web/core/modules/big_pipe/src/StackMiddleware/ContentLength.php(32): Drupal\Core\StackMiddleware\ContentLength->handle()
#18 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(118): Drupal\big_pipe\StackMiddleware\ContentLength->handle()
#19 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(92): Drupal\page_cache\StackMiddleware\PageCache->pass()
#20 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle()
#21 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#22 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(53): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#23 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(54): Drupal\Core\StackMiddleware\AjaxPageState->handle()
#24 /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php(745): Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
#25 /var/www/html/web/index.php(19): Drupal\Core\DrupalKernel->handle()
#26 {main}

Remaining tasks

  • Agree expected behaviour when no result is produced.
  • Add defensive handling in ToolBase.
  • Add test coverage for this scenario.

User interface changes

None.

API changes

None intended.

Data model changes

Issue fork tool-3571742

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

yautja_cetanu created an issue. See original summary.

ajits made their first commit to this issue’s fork.

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Assigned: Unassigned » ajits

I ran into this while working on the tool_workspace module. Testing a potential fix.

ajits opened merge request !81

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Assigned: ajits » Unassigned
Status: Active » Needs review
michaellander’s picture

michaellander commented
Status: Needs review » Needs work

Thanks for your work on this. I agree we should set a result even when tools fail to execute, but I think we would want to do it in the execute() method instead of defaulting to it in getResult(). The reason why is I think the user should expect an error if they call $tool->getResult() without ever trying to execute it, however if they try to execute it(and it fails for any reason) and they call getResult(), they should expect a failed status(as you have) without having to catch the error.

My only concern with this approach is we would not have output values in the result. We may need to confirm we are validating output values only when result is successful.

michaellander opened merge request !82

michaellander’s picture

michaellander commented
Status: Needs work » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

michaellander changed the visibility of the branch 3571742-fatal-error-when to hidden.

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Parent issue: » #3558327: [META] Tool API - Path to Beta and Inclusion in AI

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.