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

Bot stops responding after some period #6

Open
anpryl opened this issue May 19, 2018 · 4 comments
Open

Bot stops responding after some period #6

anpryl opened this issue May 19, 2018 · 4 comments

Comments

@anpryl
Copy link
Contributor

anpryl commented May 19, 2018

Bot stops responding after ~ 1 day. Only application restart helps.
Let me know if I can help you with debugging this issue.

@fizruk
Copy link
Owner

fizruk commented May 22, 2018

Thank you for reporting this issue! I will look at it closer later this week.

@anpryl
Copy link
Contributor Author

anpryl commented May 27, 2018

I think it could be lost exception in https://github.com/fizruk/telegram-bot-simple/blob/master/src/Telegram/Bot/Simple.hs#L119 on request error

@fizruk
Copy link
Owner

fizruk commented May 27, 2018

@anpryl yes, there are actually two places in that function where exceptions might ruin things:

res <- getUpdates (GetUpdatesRequest offset Nothing Nothing Nothing)

and

mapM_ handleUpdate updates

Now when I was working on this before I was not concerned with exception handling for some reason.
For now, to fix your issue, you can simply ignore exceptions and try update in the next iteration.

However, I think it would be good to introduce a friendly way to deal with exceptions (or at least report them). The library design basically follows simple MVC like gloss, The Elm Architecture and Miso, and not one of those deals with user exceptions AFAIK (Elm does not even have those).

Perhaps we should merely add a new handler to the Bot definition:

import Control.Monad.Catch

data BotApp model action = BotApp
  { botExceptionHandlers :: [Handler BotM Action]
  , ...
  }

Then we could use it like this (untested):

shieldWith :: [Handler BotM Action] -> ClientM () -> ClientM ()
shieldWith handlers m = runBotM Nothing (BotM (lift m) `catches` handlers)

startPolling :: [Handler BotM Action] -> (Update -> ClientM ()) -> ClientM ()
startPolling handlers handleUpdate = go Nothing
  where
    go lastUpdateId = do
      ...
      res <- getUpdates (GetUpdatesRequest offset Nothing Nothing Nothing)
        `shieldWith` handlers
      ...
      mapM_ (flip runBotM (reader handleUpdate `catches` handlers)) updates
      ...

In the code above I need also to pass generated actions back for processing and probably run handlers in forks. We probably don't want to handle exceptions inside exception handlers. And we probably want to add a catch-all default exception handler to the end of botExceptionHandlers list.

@anpryl what do you think?

@anpryl
Copy link
Contributor Author

anpryl commented Jun 2, 2018

@fizruk I like the idea to have separate exception handlers for handleUpdate.
But I doubt if it needed for getUpdates. I think that getUpdates exceptions should be logged with a subsequent retry. It might be a good idea to add LoggingT to allow library users control logging destination.

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