Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Chat Trigger Node): Return empty messages if loadPreviousSession and memory not connected #11787

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OlegIvaniv
Copy link
Contributor

@OlegIvaniv OlegIvaniv commented Nov 19, 2024

Summary

This PR refactors the ChatTrigger node to improve error handling and adds some test coverage.

Changes

  • Refactored ChatTrigger class to implement INodeType instead of extending Node to be better testable
  • Moved handleFormData to a standalone function for better modularity and due to "this" naming req. of INodeType context argument
  • Added error handling for memory loading failures in chat sessions

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

• Update import paths
• Implement INodeType interface
• Move handleFormData to separate function
• Adjust webhook function implementation
• Fix error handling and response formatting to always return empty array
@@ -34,7 +36,69 @@ const allowedFileMimeTypeOption: INodeProperties = {
'Allowed file types for upload. Comma-separated list of <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types" target="_blank">MIME types</a>.',
};

export class ChatTrigger extends Node {
async function handleFormData(context: IWebhookFunctions) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just moved from the class, not new

- Test webhook setup and execution
- Add file upload handling tests
- Test previous session loading
- Mock memory and error scenarios
- Update TriggerHelpers for input connection
@OlegIvaniv OlegIvaniv force-pushed the ai-458-community-issue-n8nchat-loadprevioussession-returns-500 branch from 29bb57e to 201e510 Compare November 19, 2024 11:10
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Nov 19, 2024
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 82.14286% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...hain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts 82.14% 5 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team node/improvement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant