cofacts/rumors-line-bot

Button of choosing reply always expires

MrOrz opened this issue · 2 comments

MrOrz commented

Steps to reproduce

  1. Send the following text:

    蔡英文的堂弟 蔡世能 :在桃園成立大日本帝國,希望“重建日本政府”

    若是沒有蔡英文的認同與支持,他敢這樣幹!?

  2. See the reply list, choose any reply

Expected

User should be able to read reply after choosing reply

Actual

"Button has been expired" error is shown after choosing reply

image

MrOrz commented

Seems that context is empty after providing text that has 100% match.

image

It is supposed to have at least a data attribute and a new sessionId. No data in context will trigger this logic, responding that the button is expired.

Note that the action buttons do have sessionId in its postback data, so data is passed to choosingArticle handler correctly. Then what happens when choosingArticle returns its data?

Still needs further investigation.

MrOrz commented

Root cause

handlePostback, a function that returns {context: data}, is used in handlers that should return {data}.

  1. handleInput, handlePostback and processImage are top-level functions that returns {context, replies} for singleUserHandler to set redis context and submit replies.
  2. These functions calls handlers such as initState and choosingArticle, who returns data in {data, replies, ...} format.
  3. However, in initState and choosingArticle, in some scenarios we may return results from handlePostback.
    • The caller of initState expects a result in the form {data, replies, ...} but in this case it returns {context, replies}
    • The caller tries to grab data (which is undefined in this case) and put in context, thus the context is dropped.
  4. After context is dropped, any postback action will trigger this logic, rendering the "button is expired" error.
  5. In the unit test, we mock the return result of handlePostback all together, so that we cannot spot this issue.

Suggested fix

  1. In the context of handlers such as initState and choosingArticle, instead of calling handlePostback({data}, NEXT_ACTION), we should call the handler for the NEXT_ACTION and return its result. This ensures that the signature stays the same.
  2. We do not mock the called handler in unit test so that we can inspect the response of the called handler. Although the snapshot may overlap and changes to the called handler may incur multiple snapshot changes, we can at least see if the full response is as expected.

Impact

  • handlePostback({ data }, 'CHOOSING_ARTICLE', event, userId) In initState under condition edgesSortedWithSimilarity.length === 1 && hasIdenticalDocs
    • --> If there is only 1 identical text, the resulting buttons have no context and always yields "button is expired" error
    • Example: the example in the ticket description
  • handlePostback({ data }, 'CHOOSING_REPLY', event, userId) in choosingArticle under condition articleReplies.length === 1
    • --> if there is only 1 reply, the "current context" will be dropped. If user chooses another article in the same search session, the LINE bot will respond with "expired button" error
    • Example:
      image