-
Notifications
You must be signed in to change notification settings - Fork 19
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 image message bug #160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the fix!
Let's make the management of imageProcessingCount
flag more robust :D
src/index.js
Outdated
userId, | ||
req | ||
); | ||
imageProcessingCount--; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anything between imageProcessingCount++
and imageProcessingCount--
throws exception, imageProcessingCount--
will never be executed.
I suggest we do this to make sure imageProcessingCount--
is always called:
imageProcessingCount++;
try {
// Anything between ++ and --
} finally {
imageProcessingCount--;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks for suggestion!!
d0d54ae
to
6bb4475
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
It is working great on staging. The concurrent number 3 is pretty good; when I pass 3 images to staging chatbot, I receive memory quota exceeded error, but since it's not "memory quota vastly exceed" that triggers restart, the 3 images do process in time. I believe we cannot raise the number anymore or we will use up all memory. |
Fix image message bug, should only handle text.length >= 3
Note : text.length < 3 won't be consider as new article sent in.
Limit the number of image messages linebot can process simultaneously.
Too many image messages process simultaneously will easily cause messages timeout.