PostStreamingResponseEvent never gets triggered

MR now triggers the event, when the StreamedChatMessageIterator object gets destructed.

Test PromptJsonDecoderTest seems to fail now, because \Drupal\ai\Event\PostStreamingResponseEvent::__construct doesn't receive a request_thread_id string in its constructor.
Not sure if this value is NULL in this test by design, or by accident.
If by design, then the PostStreamingResponseEvent $request_thread_id parameter should be allowed to be nullable?

Hi @svendecabooter,
The current destructor approach — it might be causing issues since it triggers for all iterator instances, including test objects.

Would it make sense to consider moving the triggerEvent() call to the ChatOutput constructor instead? That way:

- It would only fire for actual streaming responses
- It wouldn’t interfere with test objects or other unintended cases

Let me know what you think. Thanks!

@sarvjeetsingh:

I'm not sure if that would work - I'm still tipping my toes into how all of this works...

It would be something like this, in the ChatOutput constructor?:

    if ($normalized instanceof StreamedChatMessageIteratorInterface) {
      $normalized->triggerEvent();
    }

In my initial testing, that doesn't seem to work. I then get the same error as in the tests:
TypeError: Drupal\ai\Event\PostStreamingResponseEvent::__construct(): Argument #1 ($request_thread_id) must be of type string, null given

Not sure if that's something that also needs to be fixed, or if this means this approach is not workable...

Thanks for testing the ChatOutput approach!

You're right about the NULL `request_thread_id` issue. The PostStreamingResponseEvent `$request_thread_id` parameter might need to be nullable - I think that makes sense.

We could make that constructor accept nullable string if that would help resolve this issue. That might be the right approach.

I’ve made a few updates to fix the missing triggerEvent() call:

• Made $request_thread_id nullable in PostStreamingResponseEvent to avoid test failures.
• Added a $eventTriggered flag in StreamedChatMessageIterator to prevent duplicate events.
• Added null check in triggerEvent() and improved __destruct() for safety.
• Introduced finishStreaming() method so providers can trigger the event explicitly (recommended over relying on __destruct()).

Kindly review.

I have reviewed the fix, its working fine.
test scenerio:
Followed the steps to reproduce and added a logger in \Drupal\ai\OperationType\Chat\StreamedChatMessageIterator::triggerEvent

\Drupal\ai\OperationType\Chat\StreamedChatMessageIterator::triggerEvent was triggered.

I don't think where you trigger the event will work - this will be triggered before the iteration is finished I think. If you tried it and its working, please set back to in review and I'll test it out.

So I think how we could go forward with this is:

1. This MR should be kept for the changes to the PostStreamingResponseEvent - the request_thread_id could be empty.
2. For each provider we control we will add the event after the iterator is finished for 1.1.x

That is to fix the bug in 1.1.0 branches.

For 1.2.0 and going forward:

1. We add the doIterate method into the interface as the method that should be used by providers.
2. We add it with an empty method in the base class, so its not a breaking change.
3. We add getIterator into the base class and let it yield the doIterate by default and trigger the event in the end.

This means that any provider that works with this targeting 1.2.0 will be able to just move the getIterate to doIterate method without any other changes.

Any provider that doesn't do this, still works, without the event and no breaking changes are added, but we already start moving over to a sensible architecture, where we don't just control each yield, but also what happens before and after.

In 2.0.0 we remove the empty doIterate from the base class and the getIterator fromt he interface, so providers have to update.

Follow up issue for 1.2.0 and the providers coming up.

Example of fix for OpenAI in 1.1.0: https://www.drupal.org/project/ai_provider_openai/issues/3535032 🐛 Add trigger of the poststreamevent for logging Active