NicoNex / echotron

An elegant and concurrent library for the Telegram bot API in Go.
https://t.me/s/echotronnews
GNU Lesser General Public License v3.0
363 stars 26 forks source link

Add GetWebhookHandler method #20

Closed sknr closed 2 years ago

sknr commented 2 years ago

Hi Nicolò, thanks for your awesome echotron package. I extracted your handle function for the webhook into a separate function, so that I can handle it within my own http server. Maybe it's useful for others as well.

NicoNex commented 2 years ago

Hi @sknr thanks for your nice words. What's not clear to me from the code of the pr is how you intend to use your own http server. Is your intention to pass your custom handler as a parameter to the dispatcher? Because if so some other changes are needed in the dispatcher code.

sknr commented 2 years ago

Hi @NicoNex,

I just need your handler without modification in order to pass it to my own http server. Otherwise I would have start two different servers which I do not need.

Here is an example how I would like to use your handler:

func (a *App) Start {
    dsp := echotron.NewDispatcher(token, newBot)
    api := echotron.NewAPI(token)
    _, err := api.SetWebhook("https://example.com/webhook", false, nil)
    logger.LogErrorIfExists(err)

    router := mux.NewRouter()
    router.HandleFunc("/", a.homeHandler)
    router.HandleFunc("/login", a.loginHandler)
    router.HandleFunc("/logout", a.logoutHandler)
    router.HandleFunc("/webhook", dsp.GetWebhookHandler())
    // Add static file server
    fileServer := http.FileServer(http.Dir("./static"))
    router.PathPrefix("/").Handler(http.StripPrefix("/", fileServer))
    logger.LogErrorIfExists(http.ListenAndServe(":8080", router))
}

But I'll also refactor the dispatcher to be able to use a custom http handler. BTW: Thanks for your quick reply 👍

NicoNex commented 2 years ago

Hi @sknr. Thanks for the explanation, now your use case is clear to me. The code is totally fine but I have just a question. Is there a particular reason for using the getter func (d *Dispatcher) GetWebhookHandlerFunc() func(http.ResponseWriter, *http.Request) instead of just declaring the handler function like this func (d *Dispatcher) HandleWebhook(w http.ResponseWriter, r *http.Request)?

In that case your code would look like this:

func (a *App) Start {
    dsp := echotron.NewDispatcher(token, newBot)
    api := echotron.NewAPI(token)
    _, err := api.SetWebhook("https://example.com/webhook", false, nil)
    logger.LogErrorIfExists(err)

    router := mux.NewRouter()
    router.HandleFunc("/", a.homeHandler)
    router.HandleFunc("/login", a.loginHandler)
    router.HandleFunc("/logout", a.logoutHandler)
    router.HandleFunc("/webhook", dsp.HandleWebhook)
    // Add static file server
    fileServer := http.FileServer(http.Dir("./static"))
    router.PathPrefix("/").Handler(http.StripPrefix("/", fileServer))
    logger.LogErrorIfExists(http.ListenAndServe(":8080", router))
}

Because I would prefer the second way.

sknr commented 2 years ago

That's also totally fine for me.

NicoNex commented 2 years ago

Perfect then, I'm going to merge it rn. Thanks for the contribution @sknr.