Asynchronously sending `answerCbQuery()`

[esindger]

Is your feature request related to a problem? Please describe.

Now we wait for the response from the sent menu and only then send answerCbQuery(). This takes some time, during which the loading banner is displayed in the Telegram client. For this reason, a feeling of slow work of the bot.

I believe, if answerCbQuery() will send asynchronously, it improves the user experience.

https://github.com/EdJoPaTo/telegraf-inline-menu/blob/d3c00b395aa88b6b652db778362e3a359d7ab0a5/source/menu-middleware.ts#L129-L131

Describe the solution you'd like To send answerCbQuery() before _sendMenu().

context.answerCbQuery().catch(catchCallbackOld)
await this._sendMenu(responder.menu as any, context, targetPath)

[EdJoPaTo]

I like seeing this. It brings up an interesting question.

My idea behind doing answerCbQuery afterwards was to have an "everything is done, this callback query is completely and successful handled". For example when you switch from a photo menu to one with text only the old message is deleted and the new menu is sent. While that is done in parallel the delete sometimes comes first and the new message takes a moment. The user then sees the "waiting" indication as there wasnt an answerCbQuery yet. The user has no menu and a waiting indication signaling not everything is done yet. Otherwise it would end up in a situation where the menu is just gone like the waiting indicator and the user has no new menu / message to precede with and no indication that any menu is still going to appear (soon). (The other way around also happens: new message is already there and the old message still not gone but that is not as "meh" for the user in my opinion.)

But that's something I know because I built this library. So maybe I'm preoccupied with something or am missing something. So I'm open to feedback on that. Thanks for bringing this up!

[esindger]

I see your point, it's reasonable. But in this case, you can check either current menu should be deleted or edited, right? If no need to delete the menu, can answerCbQuery be sent asynchronously?

This is an example how it can be implemented:

if(this._sendMenu === editMenuOnContext) {
  context.answerCbQuery().catch(catchCallbackOld)
}

await this._sendMenu(responder.menu as any, context, targetPath)

if(this._sendMenu !== editMenuOnContext) {
  await context.answerCbQuery().catch(catchCallbackOld)
}

[esindger]

I also have another question. if a menu handler (like do or set) returns false the answerCbQuery won't send at all and the loading indicator won't be hidden. Is it by design or a bug?

Example:

menu.interact('interaction', 'interact', {
  do: async ctx => {

      // Some code

    return false
  }
})

[EdJoPaTo]

But in this case, you can check either current menu should be deleted or edited, right? If no need to delete the menu, can answerCbQuery be sent asynchronously?

Kinda yes and no. Currently the middleware does not need to know how the menu is sent. Sending the menu is not the job of the middleware, its the job of send-menu. Separation of concerns. I would prefer keeping the logic separate.

I also have another question. if a menu handler (like do or set) returns false the answerCbQuery won't send at all and the loading indicator won't be hidden. Is it by design or a bug?

Personally: If I do update a menu afterwards I do something else. Mostly with sending a message anyway. So I mostly use a answerCbQuery('with text') in these cases so there is no need for two answerCbQuery then. Maybe it should be added to send that in any case? Whats your usecase when you do not need to update the menu but do not send some kind of message to the user (via answerCbQuery)?

[esindger]

Can the code of sending menu be placed in a separate method? This will provide an ability to extend MenuMiddleware class and override that method. So this is a way to resolve my issues without changing internal logic of the library..

protected sendMenuAndAnswerCbQuery(menu, context, targetPath) {
  await this._sendMenu(menu, context, targetPath)
  await context.answerCbQuery().catch(catchCallbackOld)
}

[EdJoPaTo]

You can specify the send menu function which normally just defaults to the editMenuOnContext function: https://github.com/EdJoPaTo/telegraf-inline-menu/blob/2898974654ba9244c40becf4c8979d9be388f236/source/menu-middleware.ts#L27-L34

[esindger]

You can specify the send menu function which normally just defaults to the editMenuOnContext function:

https://github.com/EdJoPaTo/telegraf-inline-menu/blob/2898974654ba9244c40becf4c8979d9be388f236/source/menu-middleware.ts#L27-L34

Unfortunately, it's not the option because answerCbQuery calls outside of sendMenu:

https://github.com/EdJoPaTo/telegraf-inline-menu/blob/d3c00b395aa88b6b652db778362e3a359d7ab0a5/source/menu-middleware.ts#L129-L131

[EdJoPaTo]

You can call answerCbQuery multiple times. Alternatively you can create your own MenuMiddleware and use it with the MenuTemplate from this lib.

Back to the main discussion: What happens when sendMenu fails but answerCbQuery is sent first? For example when the menuBody is created and some error is thrown? The user will see "wait is gone and nothing happened"? Is that behavior wanted as the bot (somehow) reacted upon?

[esindger]

Back to the main discussion: What happens when sendMenu fails but answerCbQuery is sent first? For example when the menuBody is created and some error is thrown? The user will see "wait is gone and nothing happened"? Is that behavior wanted as the bot (somehow) reacted upon?

In my opinion, the progress bar does not inform the user that an error has occurred on the server. It shows that the server is processing the request. Although, in fact, the request has already been processed. Therefore, it is incorrect to show a stuck progress bar when request failed and nothing will load.

By the way, I've never faced errors when previous menu deleted but the new one failed to create.

[esindger]

Alternatively you can create your own MenuMiddleware and use it with the MenuTemplate from this lib.

I can't extend MenuMiddleware and override the middleware method properly, because the _responder property is private. Can you make it protected?

[EdJoPaTo]

Extending classes is normally a bad idea in most languages. Duplicating a class and changing it has way less possible errors.

Back to the topic: the error i meant can be from the creation of the menu content like renderBody or renderKeyboard. If the dev using this lib throws an Error somewhere the send menu stuff is aborted. Currently the answerCbQuery will also not be done. This is what I meant.

In the case of an error the error is thrown and the user will never see the waiting indicator vanishing. Personally I would then assume something is wrong.

Changing this to doing the answerCbQuery first would end up in the waiting indicator vanishing. Then the error happens and the menu is not changed because of the error. The user then sees the waiting indicator gone and nothing happened.

Also think about an action taking 5 Seconds. Hitting a button and then something calculated for 5 Seconds. In that case the waiting indicator would vanish. Then 5 seconds later the menu updates. As a user not knowing that it might take some time I would hit that button again as nothing happened. The best in that case would be some context.answerCbQuery('this may take a moment…'). In that case the one from the user and from this lib would be at the same time. One will work, one will be just ignored. And as we know the internet and best effort: probably the wrong one will be ignored.

The more I think about that I think that this maybe shouldnt be done at all from this lib. The dev knows best when and with what message that callback query should be answered in the specific case…

[EdJoPaTo]

This is a relevant commit to this issue: 3ba6f62f56d3b0008f9335ffd98a782a96e69197

Seeing that commit I remember why I changed that. One of my bots has photo menus displaying images in the menu body. These images differ and are 1: not cached by Telegram yet or 2: not cached by the client yet. In the first case telegram deletes the old message and starts to download the image from the url. When that is done the message is sent to the client. Then the first and second case apply. If the client never had that image yet the client downloads the image preview and displays the message. (Then when the message is there the full image is loaded by the client).

Especially in the first case when telegram still needs to load the image this takes a moment. Displaying a progress bar in that case sounds correct to me. The official Telegram docs no not point out a wanted behavior here.

Another thing I noticed while testing: When answerCbQuery is called two times in a row (first with undefined, second with a text) some clients show the text, some clients ignore the second answerCbQuery.

Removing answerCbQuery from the middleware logic would not be ideal as the menuBody can also be called from commands which do not have a callbackQuery which could be answered. This would result in additional logic in all menu bodies.

Given all that… I think it is still the best thing to show the waiting indicator until everything is done. When functions like do or set are used, the callback Query can be answered early (and should be when the menu isnt updated afterwards) to indicate that the action might take a moment.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.