tucnak/telebot

proposal: add context.Context support

AH-dark opened this issue · 11 comments

In the process of using in a production environment, we often need to trace the source and operation of each request, that is, link tracing.

In the current scenario, although telebot has implemented a handler mode similar to http, it is completely unadaptable to link tracing. It would be much better if we just add context.Context to the Update object or try to pass messages not through Update but directly through *Context.

I think this is a problem worth considering. When I tried to combine this package with microservice systems and message queues to improve message processing efficiency and concurrency capabilities, I found that it could not associate the message context, which is fatal.

Inspired by go-chi/chi, I attempted to redesign handlers and middlewares to accept the stdlib context.Context. You can find the changes in the following GitHub repository: https://github.com/MikhailChe/telebot/tree/mikhailche-context-first-effort

I am currently testing it, and at the moment, it feels a bit strange. I am considering moving telebot.Context as a value inside the context.Context with some helper functions to retrieve it from there.

Unfortunately, this redesign requires rewriting all currently implemented handlers and middlewares and might remain as a separate project indefinitely (or perhaps become part of telebot v4? :))

I suggest you follow the example of github.com/cloudwego/hertz. Pass context.Context as the first parameter and telebot.Context as the second parameter.

demget commented

Related to #222

@demget Do I need to provide an implementation?

demget commented

I don't really like the idea of pasting the context everywhere by default, it's not that useful for small single bots, and we must keep the backward compatibility.

It would be great to define the main points on where and why the context should be included, and think of how can we provide a clean non-breaking implementation.

Your help is greatly appreciated. I'm adding this issue to the future releases milestone.

demget commented

@MikhailChe @AH-dark do you have any solid examples of using the forked telebot versions with context.Context included?

@MikhailChe @AH-dark do you have any solid examples of using the forked telebot versions with context.Context included?

I agree with your view, but it needs to be pointed out that adding a context would not be a burden for the small bot, whereas not adding a context would prevent us from using telebot to build large-scale robots.

Additionally, as far as I know, people usually use Python instead of Go to build small bots. 🤔

@demget, I have a small pet project that uses a forked version for tracing. This allows me to pass "spans" via context to the http client. It's a bit sloppy, but it is a real working example.