fix(Google Gemini Node): Filter out undefined parts when processing API responses#28567
Conversation
Performance ComparisonComparing current → latest master → 14-day baseline docker-stats
Idle baseline with Instance AI module loaded
Memory consumption baseline with starter plan resources
How to read this table
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
Architecture diagram
sequenceDiagram
participant Engine as n8n Workflow Engine
participant Node as Google Gemini Node
participant GoogleAPI as Google Gemini API
participant Tools as n8n Tools / Functions
Engine->>Node: execute()
loop Until Max Iterations
Node->>GoogleAPI: Generate content request
GoogleAPI-->>Node: GenerateContentResponse (JSON)
Note over Node: NEW: Process Candidates safely<br/>(Handles undefined/null objects)
Node->>Node: CHANGED: getToolCalls()<br/>Filter parts via isFunctionCallPart()
alt Tool calls detected
loop For each Tool Call
Node->>Tools: Execute identified tool
Tools-->>Node: Tool output
end
Note over Node: Add tool results to message history
else No tool calls
Note over Node: Break loop
end
end
Note over Node: NEW: Build Final Response
opt options.simplify & options.includeMergedResponse
Node->>Node: CHANGED: Filter text parts via isTextPart()<br/>(Skips undefined parts to prevent crash)
end
Node-->>Engine: Return INodeExecutionData[]
michael-radency
left a comment
michael-radency
left a comment
There was a problem hiding this comment.
LGTM, added some non-blocking comments
| contents.push.apply( | ||
| contents, | ||
| response.candidates.map((c) => c.content), | ||
| ); |
There was a problem hiding this comment.
claude:
push.apply offers no performance advantage over spread in modern JS engines. Both translate to the same operation: passing array elements as individual arguments to push.
The old push.apply pattern came from pre-ES6 days when spread didn't exist. It was used as a workaround. Today, contents.push(...arr) is the idiomatic equivalent and compiles to the same bytecode in V8.
There was a problem hiding this comment.
I think this got changed because I ran eslint autofix, and we have a lint rule to prefer apply over spread
The reasoning: if the array being spread is very big, it can cause stack overflow, because each array item is converted into a function argument, therefor it takes a lot of memory on the stack
This doesn't apply here, because contents is always rather small, but I'll leave it like this to keep the linter happy
| it('should skip undefined parts when parsing tool calls', async () => { | ||
| executeFunctionsMock.getNodeParameter.mockImplementation((parameter: string) => { |
There was a problem hiding this comment.
maybe better name
"should handle undefined entries in parts array"
There was a problem hiding this comment.
Agree, updated it
…e-typeversion-11-crashes
…e-typeversion-11-crashes
…e-typeversion-11-crashes
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
RomanDavydchuk
deleted the
node-4822-community-issue-googlegemini-node-typeversion-11-crashes
branch
|
Got released with |
2 participants
Summary
If Google's API response contained undefined parts, for some reason, the node would fail because
'functionCall' in pthrows an error whenpis undefined. This PR adds extra checks to make sure we properly filter out unwanted parts of the responseRelated Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/NODE-4822/community-issue-googlegemini-node-typeversion-11-crashes-with-cannot
Closes #28215
Review / Merge checklist
Backport to Beta,Backport to Stable, orBackport to v1(if the PR is an urgent fix that needs to be backported)