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

bug: aws-lambda webhookCallback adapter doesn't work #412

Closed
chrisandrewcl opened this issue May 22, 2023 · 6 comments
Closed

bug: aws-lambda webhookCallback adapter doesn't work #412

chrisandrewcl opened this issue May 22, 2023 · 6 comments

Comments

@chrisandrewcl
Copy link
Contributor

chrisandrewcl commented May 22, 2023

I've started experimenting with grammyjs and encountered a few issues while trying to use the aws-lambda adapter for the webhookCallback:

  1. webhookCallback returns an async function, which causes the lambda to be called without the callback, resulting in "callback is not a function" errors.
  2. Webook replies don't work without explicitly setting Content-Type: application/json.
  3. Webhook replies work only for the first method call, which is confusing.

I was able to work around issues 1 and 2 by setting the header directly and wrapping the handler with a promise:

export const handler = async (event, ctx) => {
  let result = {};

  try {
    result = await new Promise((resolve) => {
      webhookCallback(bot, 'aws-lambda')(event, ctx, (error, response) => {
        if (error) {
          console.error(error);
        }
        resolve(response);
      });
    });
  } catch (error) {
    console.error(error);
  }

  /* Needed when replying directly to the webhook request */
  if (result?.body) {
    result.headers = {
      ...result.headers,
      ...{ 'Content-Type': 'application/json' },
    };
  }

  return result;
});

If my understanding is correct, the framework could be improved by:

  1. Supporting async/await.
  2. Setting the header when using webhook replies.
  3. I guess changing how the webhookCallback work would spill in a lot of other places and could be too much to ask, but it would be great to have more flexibility by having something as const reply = ctx.reply(/* ... */, { returnJsonPayload: true }) (disregarding canUseWebhookReply) so that we can explicitly set a response after we have done everything else. Is something like this possible in the current version? Without it, I am considering "leaking" the function given to canUseWebhookReply so that we can control when it should return true later on each request, but it seems messy. I hope there is a better way.

1 and 2 look like good "first issues", so I would like to contribute with a PR if that's ok.

@KnorpelSenf
Copy link
Member

Absolutely! I'm looking forward to your contributions :)

I believe @amberlionk to be the one who contributed the AWS adapter in #62. In case you have any questions about the implementation, I'm sure we are both open to answer them!

@KnorpelSenf
Copy link
Member

Sorry for the late reply btw, I was a little busy recently

@KnorpelSenf
Copy link
Member

@all-contributors add @chrisandrewcl for bug

@allcontributors
Copy link
Contributor

@KnorpelSenf

I've put up a pull request to add @chrisandrewcl! 🎉

@KnorpelSenf KnorpelSenf changed the title aws-lambda webhookCallback adapter doesn't work bug: aws-lambda webhookCallback adapter doesn't work May 28, 2023
@chrisandrewcl
Copy link
Contributor Author

@KnorpelSenf I've submitted a PR for review, addressing points 1 and 2.

Regarding point 3, is there any way to call an API method on "dry-run" to get only the JSON payload?

If not, and if it is something that makes sense for the framework, I can submit a separate feature request (to be considered for a future version) and this issue can be hopefully closed with PR#418.

@KnorpelSenf
Copy link
Member

Regarding point 3, is there any way to call an API method on "dry-run" to get only the JSON payload?

I'm not 100 % sure I understand what you mean, but I believe https://grammy.dev/advanced/transformers.html to be the solution.

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