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

Idempotency Concern of the ChatFunction #8

Open
NullPointer4096 opened this issue Apr 16, 2023 · 1 comment
Open

Idempotency Concern of the ChatFunction #8

NullPointer4096 opened this issue Apr 16, 2023 · 1 comment

Comments

@NullPointer4096
Copy link

Description:

I would like to kindly bring attention to a potential issue in the ChatFunction, which is invoked by the chatRule IoT TopicRule. This function calls the IotData.publish to send chat messages to the frontend, where the frontend appends the new messages with previous ones. However, this message sending is not idempotent. Suppose the IoT-triggered function crashes after successfully sending a chat message to the frontend-facing IoT topic (but before acknowledging the event source that the function has finished). In that case, when the function is retried, another message will be sent and received by the frontend.

Suggested Fix:

Although there's no serious harm in sending two identical chat messages, this duplicate sending due to untimely retries can be prevented. Specifically, one can attach a unique id to each chat message when the message is being generated. Before the frontend appends the new message to the message list, it should check whether the message id is already present in a deduplication set for holding all previous chat message ids. If the id is already present, don't append the message. Otherwise, append the message as usual.

Thank you for considering this feedback. I hope that this suggestion could help improve the idempotency and user experience of the ChatFunction. Please feel free to reach out if you have any questions or concerns.

@danilop
Copy link
Owner

danilop commented Apr 18, 2023

Thanks, that's a very good point! If you'd like to contribute a PR with your suggested fix, I'd be happy to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants