Problem/Motivation
There are some cases (in my case, when there was an embedding error coming from the server) - when the `$event->getInput()` returns a string instead of an object.
This needs to be handled carefully (instead it explodes now in a fatal error).
Issue fork langfuse-3563390
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:
Comments
Comment #2
nikro commentedPushed a fix for this. The subscriber assumed
$event->getInput()always returns an object, and called bothget_class()andtoString()on it unconditionally. Both are fatal on a string, and theget_class()line comes first, so it fails before reaching the call that looks like the obvious culprit.The same assumption appeared at four call sites, so rather than guarding each one the input is now normalised once near the top of the handler and that value is used everywhere below. Object, string,
__toString, scalar and array are all handled, andget_class()becomesget_debug_type(), which describes a non-object rather than raising a TypeError. Four scattered guards would have left whoever adds a fifth call site to reintroduce the same bug.Covered by unit tests over every shape the AI module is known to produce, including the plain string from your embedding error. I checked they fail without the fix rather than assuming: reverting the string branch produces two failures, restoring it returns to green. 21 tests, 106 assertions, phpcs clean, phpstan unchanged.
One note for whoever merges: this touches the same line of
onPreGenerateResponse()as #3594090, so whichever lands second will need a one line conflict resolved. Both changes are kept, the normalised input variable together with the extra arguments that issue adds.Comment #5
abhisekmazumdarReviewed MR !7 and pushed one small follow-up commit, fixing the cspell failures in the new test file. That's the only CI failure this MR actually caused; the other failing job (
phpstan) is unrelated, more on that below.I'm marking this RTBC. Reasoning:
normalizeInput()correctly replaces all four previously-unguardedgetInput()call sites.toString()is still checked first, same as before.get_debug_type()mirrorsget_class()'s prior output for objects, and safely describes non-objects instead of fataling.__toString, scalar, array, and null inputs.phpstanfailure on 1.x. I reproduced it independently againstorigin/1.xHEAD, so it's not this issue's problem to fix.One small note: the MR description mentions 21 tests / 106 assertions, but the test file as it stands runs 8 tests / 11 assertions. Worth a quick correction to the description, not a blocker.
Comment #6
nikro commented