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

Updates for outgoing messages #32

Open
spontanurlaub opened this issue Dec 22, 2020 · 18 comments
Open

Updates for outgoing messages #32

spontanurlaub opened this issue Dec 22, 2020 · 18 comments
Labels
enhancement New feature or request

Comments

@spontanurlaub
Copy link
Collaborator

spontanurlaub commented Dec 22, 2020

Userbots usually work by reacting to outgoing messages from other clients on the same account. This is currently not possible with this API. I suggest to add a command like /setOption?name=receiveOutgoingUpdates&value=1 and a binlog database to store these client settings in JSON format. This database can then also be used if you want to receive updates for changing online statuses or drafts. Just adding new update types and sending updates for outgoing messages may not be backwards compatible, so adding this setting per client is probably the way to go.

@MarcoBuster MarcoBuster added the enhancement New feature or request label Dec 24, 2020
@andrew-ld
Copy link
Member

could lead to recursion problems between softwares

@spontanurlaub
Copy link
Collaborator Author

@andrew-ld That's why it must be enabled explicitly. It should not be a default behavior

@watzon
Copy link

watzon commented Feb 1, 2021

So I've been doing some digging around because I really want to use this to make userbot, but not having outgoing message support is obviously a deal breaker. I added an is_outgoing property to Message and commented out the section in need_skip_update_message that checks if the message is outgoing and returns true (in most cases) if it is.

After doing a simple ping test I noticed that the command message is considered outgoing, but the reply isn't even though they're both coming from me. This would seem to make it pretty obvious which messages are meant to be outgoing and which aren't. A simple check of is_outgoing prior to acting on a command and some common sense should keep recursive commands from being too much of a problem.

Tbh I don't feel like it's the job of the library to prevent recursive commands though, but that's my opinion.

@spontanurlaub
Copy link
Collaborator Author

Sorry that I'm currently not working on this, have a bunch of exams in February. I will have a look at it again when I'm done with them.

@penn5
Copy link

penn5 commented Feb 2, 2021

So I've been doing some digging around because I really want to use this to make userbot, but not having outgoing message support is obviously a deal breaker. I added an is_outgoing property to Message and commented out the section in need_skip_update_message that checks if the message is outgoing and returns true (in most cases) if it is.

After doing a simple ping test I noticed that the command message is considered outgoing, but the reply isn't even though they're both coming from me. This would seem to make it pretty obvious which messages are meant to be outgoing and which aren't. A simple check of is_outgoing prior to acting on a command and some common sense should keep recursive commands from being too much of a problem.

Tbh I don't feel like it's the job of the library to prevent recursive commands though, but that's my opinion.

The library should have is_outgoing and is_from_this_session, where the latter indicates that the update was received in response to an api request that we made

@watzon
Copy link

watzon commented Feb 2, 2021

@penn5 that I can definitely agree with

@JosXa
Copy link

JosXa commented Feb 9, 2021

could lead to recursion problems between softwares

I guess a circuitbreaker would be waaay out of scope

@JosXa
Copy link

JosXa commented Feb 9, 2021

So I've been doing some digging around because I really want to use this to make userbot, but not having outgoing message support is obviously a deal breaker. I added an is_outgoing property to Message and commented out the section in need_skip_update_message that checks if the message is outgoing and returns true (in most cases) if it is.

After doing a simple ping test I noticed that the command message is considered outgoing, but the reply isn't even though they're both coming from me. This would seem to make it pretty obvious which messages are meant to be outgoing and which aren't. A simple check of is_outgoing prior to acting on a command and some common sense should keep recursive commands from being too much of a problem.

Yep, one particular logged-in session never receives its own outgoing requests back as updates, instead those are provided by mtproto in the method return values.

This distinction is made by every single userbot API out there, and usually it boils down to pretty much what you described.
The concern about infinite loops could in my eyes only occur naturally iff you have 3 or more programmatic clients in the loop, running on the same session - _which, why would anyone do that. _

With two clients, they might see each others' messages, but apart from them getting stuck in an intentional echo loop I don't see it as a big deal.

Tbh I don't feel like it's the job of the library to prevent recursive commands though, but that's my opinion.

However, you could also force this behavior with a single client that "re-injects" its own updates received as command responses from the server back into its own update queue.
For that, I see a few use cases but I haven't seen any library go for that as of yet and you'd really have to shoot yourself in the foot as a lib maintainer if you think that's a good default ^^

Jog my memory here for a sec, does tdlight allow multiple sessions of the same account at once? Because only in that case would you have to be a little careful here.

That being said, I'm despetately waiting for this and since I'm not a cpp dev I even started making strange workarounds where the updates come from a tdlib wrapper in my language which then dispatches them to the app but then the rest of the communication happens via bot api...
Suffice to say I am very much willing to help out in any way I can, just let me know if there's something I can do.

Oh, and one more thing:

The is_outgoing property has always been bothering me. The fact that in 50% of userbot implementations you can trigger /commands in chat with yourself, while the other half either has special constructs to detect that case or straight up doesn't work, tells me that something weird is going on.
I'm not 100% certain where this flag comes from (not mtproto but tdlib?),
but let me tell you, it is extremely inconsistent and confusing when applied practically. Whenever you design a command, you need to keep in mind the special behavior of your personal chat and whatever library you're working with right now.
Maybe we can find a way around that, but my clear opinion is that for any practical use of the is_outgoing property, it must be true in your cloud chat.

@JosXa
Copy link

JosXa commented Feb 9, 2021

So @penn5 to my knowledge there's no way this happens and no need to handle it

@penn5
Copy link

penn5 commented Feb 9, 2021

So I've been doing some digging around because I really want to use this to make userbot, but not having outgoing message support is obviously a deal breaker. I added an is_outgoing property to Message and commented out the section in need_skip_update_message that checks if the message is outgoing and returns true (in most cases) if it is.
After doing a simple ping test I noticed that the command message is considered outgoing, but the reply isn't even though they're both coming from me. This would seem to make it pretty obvious which messages are meant to be outgoing and which aren't. A simple check of is_outgoing prior to acting on a command and some common sense should keep recursive commands from being too much of a problem.

Yep, one particular logged-in session never receives its own outgoing requests back as updates, instead those are provided by mtproto in the method return values.

This distinction is made by every single userbot API out there, and usually it boils down to pretty much what you described.
The concern about infinite could in my eyes only occur naturally iff you have 3 or more programmatic clients in the loop, running on the same session - which why would anyone do that.

With two clients, they might see each others' messages, but apart from them getting stuck in an intentional echo loop I don't see it as a big deal.

Tbh I don't feel like it's the job of the library to prevent recursive commands though, but that's my opinion.

However, you could also force this behavior with a single client that "re-injects" its own updates received as command responses from the server back into its own update queue.
For that, I see a few use cases but I haven't seen any library go for that as of yet and you'd really have to shoot yourself in the foot as a lib maintainer if you think that's a good default ^^

Jog my memory here for a sec, does tdlight allow multiple sessions of the same account at once? Because only in that case would you have to be a little careful here.

That being said, I'm despetately waiting for this and since I'm not a cpp dev I even started making strange workarounds where the updates come from a tdlib wrapper in my language but then works via proxied REST for communicating with Telegram...
Suffice to say I am very much willing to help out in any way I can, just let me know if there's something I can do.

Oh, and one more thing:

The is_outgoing property has always been bothering me. The fact that in 50% of userbot implementations you can trigger /commands in chat with yourself, while the other half either has special constructs to detect that case or straight up doesn't work, tells me that something weird is going on.
I'm not 100% certain where this flag comes from (not mtproto but tdlib?),
but let me tell you, it is extremely inconsistent and confusing when applied practically. Whenever you design a command, you need to keep in mind the special behavior of your personal chat and whatever library you're working with right now.
Maybe we can find a way around that, but my clear opinion is that for any practical use of the is_outgoing property, it must be true in your cloud chat.

Frankly, it's not good practice to see if the message was sent by your current code, because telegram doesn't guarantee that all or only updates from a request are sent with its response. The fact that others do it does not make it cleaner. The ideal situation is that userbots check the random ID on the message against their pending outgoing messages (telekram). But that's not really plausible here, so outgoing and from_request it is. And a circuit breaker is not feasible since the requests are opaque.

@penn5
Copy link

penn5 commented Feb 9, 2021

So @penn5 to my knowledge there's no way this happens and no need to handle it

No way what happens?

@JosXa
Copy link

JosXa commented Feb 9, 2021

So @penn5 to my knowledge there's no way this happens and no need to handle it

No way what happens?

I'm not sure, maybe tdlib does things differently than how I'm used to from e.g. Pyrogram/Telethon/Madeline 🤔
Does it indeed dispatch those mini-updates received from command executions to the current client? That would be odd, as they normally only contain status updates about new users, the sent message, etc. which are just meant to be added to the internal cache for later lookup.
Or am I completely missing the point why everyone's concerned about recursion and the need for that "comes_from_this_bot_api" field?

Frankly, it's not good practice to see if the message was sent by your current code, because telegram doesn't guarantee that all or only updates from a request are sent with its response. The fact that others do it does not make it cleaner.

I can't find a claim of someone doing this in the conversation, what are you referring to exactly? :)

@penn5
Copy link

penn5 commented Feb 10, 2021

So @penn5 to my knowledge there's no way this happens and no need to handle it

No way what happens?

I'm not sure, maybe tdlib does things differently than how I'm used to from e.g. Pyrogram/Telethon/Madeline 🤔
Does it indeed dispatch those mini-updates received from command executions to the current client? That would be odd, as they normally only contain status updates about new users, the sent message, etc. which are just meant to be added to the internal cache for later lookup.
Or am I completely missing the point why everyone's concerned about recursion and the need for that "comes_from_this_bot_api" field?

Frankly, it's not good practice to see if the message was sent by your current code, because telegram doesn't guarantee that all or only updates from a request are sent with its response. The fact that others do it does not make it cleaner.

I can't find a claim of someone doing this in the conversation, what are you referring to exactly? :)

I have no clue what you're talking about at this point. Telekram dispatches all updates, and that's fine. It allows proper consistency. Idk what tdlib does.

@luckydonald
Copy link
Collaborator

luckydonald commented Feb 10, 2021

Logically, if all those apps they release are based on tdlib, those own (other-device) messages must be there, otherwise you wouldn't see them suddenly appear in your chats. For bots instead of users, I don't know how this behaves.

While I think having is_outgoing and is_from_this_session totally make sense,
we should also make sure the api can (per default) behave compatible to the usual bot api. That means to allow not sending own/outgoing messages.
Otherwise we might break implementations easily. One of the use cases of this projects is taking a simple normal bot and plugging it into a user account.

It would be great if instead setWebhook and getUpdates could have an option for that, similar to allowed_updates.
Like allow_own_messages Boolean (defaulting to false).
That would enable all this planned goodness, while still allowing to be used as a drop-in replacement for the official API.

@JosXa
Copy link

JosXa commented Feb 10, 2021

Could someone explain the purpose of is_from_this_session again please?

@JosXa
Copy link

JosXa commented Feb 10, 2021

allow_own_messages

How about receive_outgoing instead?
We're not allowing anything, right?

@luckydonald
Copy link
Collaborator

Sure. Even better. The feature is more important, the actual name is not as relevant.
That is, to keep compatibility with the normal API and such features which could break bots as opt-in.

@luckydonald
Copy link
Collaborator

Could someone explain the purpose of is_from_this_session again please?
.~ @JosXa

The purpose of is_from_this_session is to distinguish of actual messages send by the Bot API client ("we", TDLight) and any other connected client. That way you can choose to ignore the messages which are also returned by sendMessage and other send* methods (they would have that as true) and say your mobile phone connected to the same user.

A lot of user bots are running for a normal user, in addition to them using the account. That way they can send a command, and the program can then act on it. Usually replacing the message content with something else or running an action and deleting the message. In the wild, often it looks like !command do stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants