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

Async/await works with polling, but not with webHooks #567

Closed
dmbaranov opened this issue Dec 6, 2018 · 6 comments
Closed

Async/await works with polling, but not with webHooks #567

dmbaranov opened this issue Dec 6, 2018 · 6 comments

Comments

@dmbaranov
Copy link
Contributor

Context

I have a bot and I'm running it with the different update approaches - long polling and webHooks. For some reasons, the same code works with the first option but doesn't work with the second one. After some investigation, I found out that this piece of code breaks webHooks:

myStage.enter(async ctx => {
  const someData = await makeRequest();
  ctx.reply('Hello');
});

What's strange about that, if I'll have 2 ctx.reply(), then it will work and bot will send both messages. But if there is only one ctx.reply(), it won't send anything.

  • Telegraf.js Version: 3.25.0
  • Node.js Version: 9.7.0
  • Operating System: Ubuntu 16.04

Expected Behavior

The same code should work both with long polling and webHooks.

Current Behavior

Apparently, it doesn't.

Failure Information (for bugs)

There are no any errors.

Steps to Reproduce

  1. Create a stage;
  2. Use promise in enter callback.
@dmbaranov dmbaranov changed the title Async/await works with polling, but not webHooks Async/await works with polling, but not with webHooks Dec 6, 2018
@dmbaranov
Copy link
Contributor Author

After further investigation, I found out one more interesting piece. I have a helper function:

const asyncWrapper = fn => {
  return function(ctx, next) {
    fn(ctx).catch(error => {
      console.log('error');
      return next();
    });
  };
};

and I use it this way:
bot.command('enterStage', asyncWrapper((ctx: any) => ctx.scene.enter('myStage')));

Changing it to

const asyncWrapper = fn => {
  return async function(ctx, next) {
    try {
      await fn(ctx);
    } catch (error) {
      console.log('error');
      return next();
    }
  };
};

fixes the problem. Nevertheless, I consider it as a bug because this code works fine with long polling.

@tva10
Copy link

tva10 commented Dec 6, 2018

@dmbaranov, Hello!

Not sure how your await works, but I think that here your promise chain is broken
Try to check your problem code with enabled Webhook and disabled Webhook reply

const config = {
	telegram: { webhookReply: true } // default true, but need to set false
};
const app = new Telegraf( /*token*/, config);

And so another moment, that reply method is async too. I think that it needs set await prefix

myStage.enter(async ctx => {
  const someData = await makeRequest();
  await ctx.reply('Hello');
});

ref: #320 #497

@dmbaranov
Copy link
Contributor Author

@tva10 hi. Pretty much, asyncWrapper function wraps some methods as I showed above, this way I can skip try-catch blocks in stage controllers.

And actually, the issue is not that my promise chain is broken, it's about that it works with long polling approach but doesn't work with webhook. I didn't make any changes to my code, just changing from bot.startPolling() to bot.startWebhook() and this only change breaks stage's entrance callback.

Btw, I have 2 different stages:

  1. stageOne.enter(async ctx => {...}
  2. stageTwo.enter(ctx => {...}

Whereas they both works fine with long polling approach, stageOne won't work with webhooks. Presumably because it has async callback.

But again, what I'm trying to say is, the same code works differently depending on an update approach.

@tva10
Copy link

tva10 commented Dec 6, 2018

@dmbaranov yep, I tried to hint that code (with broken promises) works differently too (in #320)

I definitly think that in your promise chain something went wrong
Did you try disable webhookReply in config? (I miss in my example that it need to be set false for disable)
If that will revive broken code that means what I said before.

By default webhook, on update, keep opened https socket as long as promise has pending state or timeout happens. This soket will be used for your first answer/reply.
And this does not happen in polling mode or with disabled webhookReply

And checked your first Async Wrapper function, I think it needs more returns

const asyncWrapper = fn => {
  return function(ctx, next) {
    return fn(ctx).catch(error => {
      console.log('error');
      return next();
    });
  };
};

@dmbaranov
Copy link
Contributor Author

@tva10 thanks for reply. Setting webhookReply: false fixes the problem indeed. Thanks for the explanation, though it's still not clear enough for me why this is happening..
And thanks for the notice about asyncWrapper function as well :)

I'll mark ticket as closed.

@gildniy
Copy link

gildniy commented Oct 9, 2020

For anyone who likes one line code:
const asyncWrapper = fn => (...args) => fn(...args).catch(args[2]);

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants