sochix / TLSharp

Telegram client library implemented in C#
1.01k stars 380 forks source link

Constructor 1307772980 or 0x‭4DF30834‬ #929

Closed solarin closed 4 years ago

solarin commented 4 years ago

Hello, i spent half a day to figure out why:

  1. Telegram is responding with this constructor even though it's not documented anywhere
  2. During the TelegramClient.ConnectAsync Telegram responds with this constructor, but TLSharp is somehow recovering from the exception thrown by not finding the type
  3. I sorted out the problem duplicating the TLChannel constructor 1588737454

The question is:

  1. Why is TLSharp not implementing this constructor?
  2. Is it normal to have constructors like this one with no documentation anywhere?

Proposals:

  1. Implement the constructor
  2. catch and handle the exceptions of missing types not only in one function, but in any place where processMessage is called. Now it's handled only in HandleContainer

cheers

knocte commented 4 years ago

During the TelegramClient.ConnectAsync Telegram responds with this constructor

I don't understand the sentence above. ConnectAsync is a method, how can a method "respond with a constructor"? Are we lost in translation here?

catch and handle the exceptions of missing types not only in one function, but in any place where processMessage is called.

Exception of missing type? It seems you got an exception and you're not sharing the details of it. Please paste the ex.ToString() result of it.

solarin commented 4 years ago

you are right, didn't explain it properly, i was too tired :D

Exception of missing type? It seems you got an exception and you're not sharing the details of it. Please paste the ex.ToString() result of it.

The given key was not present in the dictionary.

ConnectAsync is a method, how can a method "respond with a constructor"?

When I run the code, the first thing I do is "ConnectAsync" if not connected. If the previous communication was interrupted, Telegram sends the updates that I lost, included the one that triggered the above exception.

The exception is here: ConnectAsync -> MtProtoSender.Receive -> MtProtoSender.processMessage -> HandleContainer -> MtProtoSender.processMessage -> HandleRpcResult -> TLRequestInvokeWithLayer.DeserializeResponse ->ObjectUtils.DeserializeObject -> TLUpdates.DeserializeBody -> Updates and Users are deserialized succesfully When it comes to desrialize the Chats inside the TLUpdates...

Chats = (TLVector<TLAbsChat>)ObjectUtils.DeserializeVector<TLAbsChat>(br);

ObjectUtils.DeserializeVector -> TLVector.DeserializeBody

Type type = TLContext.getType(constructor);

with constructor = 1307772980

Exception "The given key was not present in the dictionary"

and it is handled in the try-catch in HandleContainer

So, this exception of missing type is handled in this circumstance (processMessage calls HandleContainer), while it is not handled if processMessage calls any other methods that fall back on processMessage itself, such as HandleRpcResult and an exception is thrown.

shouldn't it be the case to handle exceptions of missing type everywhere?

solarin commented 4 years ago

and, even more important, why the heck this constructor is given as response (passed by, call it the way you want) by Telegram even though it's not documented?

This missing constructor basically is equivalent to -1588737454

knocte commented 4 years ago

The exception is here:

That's not pasing ex.ToString(), please paste exactly this.

solarin commented 4 years ago

it is!

knocte commented 4 years ago

It is not. When you paste just what ex.ToString() gives you, I would see: first the exception type, after that, the exception message, and after that the stacktrace and inner exceptions. I don't see this format in your paste.

solarin commented 4 years ago
Untitled
System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at TeleSharp.TL.TLContext.getType(Int32 Constructor) in  repos\TLSharp\TeleSharp.TL\TLContext.cs:line 28
   at TeleSharp.TL.TLVector`1.DeserializeBody(BinaryReader br) in  repos\TLSharp\TeleSharp.TL\TLVector.cs:line 90
knocte commented 4 years ago

What version of the nuget package are you using when you got this exception?

solarin commented 4 years ago

nuget package for TLSharp or what?

knocte commented 4 years ago

Yeah, you're filing the bug on TLSharp repo. But if you're using a git clone instead of using the nuget package when you got this, what commit are you at? the last one in the master branch?

solarin commented 4 years ago

i forked at here: 6d2c77f5

Ah.. wait, i'm receiving this type in response to a newly implemented method https://core.telegram.org/method/channels.editBanned

which, anyway, returns Updates (which we have as implemented type)

knocte commented 4 years ago

i forked

You cannot report a bug against a repo that happens in a fork whose diff is unknown (you would need to share your diff first too). Otherwise we don't have the whole picture.

I think you may need to add a call to .Add() manually in the static constructor of TLContext, but I'm not sure cause I'm not familiar with this part of the code. Maybe @aarani can chime in?

solarin commented 4 years ago

i will add this changes in response to this issue

i sorted it out, cloning the TLChannel class and changing its constructor number to the one in the subject.

my question was different, how's it possible that this constructor is not documented anywhere? @aarani , any ideas?

knocte commented 4 years ago

i will add this changes in response to this issue

But, if this work is related to https://core.telegram.org/method/channels.editBanned , I have to ask first: is this a thing not covered in Layer 66, but covered in 108? if yes, then you should base your work on top of the Layer-update PR.

solarin commented 4 years ago

ok. honestly i am a bit confused on how to see which methods are supported by which layers. a quick hint?

aarani commented 4 years ago

Telegram servers thinks you're using layer higher than 93, 0x‭4DF30834‬ is the constructor for TLChannel on those layer. you can't deserialize it using layer 66 TLChannel It can break deserialization. change back your layer to 66 or use whole layer upgrade PR on TgSharp Fork if you need newly introduced method.

i forked at here: 6d2c77f

Ah.. wait, i'm receiving this type in response to a newly implemented method https://core.telegram.org/method/channels.editBanned

which, anyway, returns Updates (which we have as implemented type)

You can't just add a new method into an old layer and expect it to work you have to update the whole layer.

knocte commented 4 years ago

how to see which methods are supported by which layers

@solarin well, check the diff of the layer-update PR? I see a new type TLRequestEditBanned type introduced there, so there's a good chance that what you're doing is already (mostly, if not 100%) covered in that PR.

knocte commented 4 years ago

You can just add a new method

@solarin please not the typo above ^, Afshin meant "can't" (and edited his comment)

solarin commented 4 years ago

i forked at here: 6d2c77f Ah.. wait, i'm receiving this type in response to a newly implemented method https://core.telegram.org/method/channels.editBanned which, anyway, returns Updates (which we have as implemented type)

You can't just add a new method into an old layer and expect it to work you have to update the whole layer.

of course, i also recreated the types.

but this constructor 130777... is not documented anywhere! that's what leaves me thrilled

knocte commented 4 years ago

of course, i also recreated the types.

Please don't do this manually, use Afshin's PR, fork from his branch.

aarani commented 4 years ago

i forked at here: 6d2c77f Ah.. wait, i'm receiving this type in response to a newly implemented method https://core.telegram.org/method/channels.editBanned which, anyway, returns Updates (which we have as implemented type)

You can't just add a new method into an old layer and expect it to work you have to update the whole layer.

of course, i also recreated the types.

but this constructor 130777... is not documented anywhere! that's what leaves me thrilled

It's documented if you know where to look https://github.com/telegramdesktop/tdesktop/blob/e10c928207189b6554295e5662c23dfc1e5450da/Telegram/Resources/scheme.tl#L232

aarani commented 4 years ago

of course, i also recreated the types.

what types did you recreated exactly?

solarin commented 4 years ago

TLRequestEditBanned 0x72796912 TLChatBannedRights -1626209256

solarin commented 4 years ago

of course, i also recreated the types.

Please don't do this manually, use Afshin's PR, fork from his branch.

i had to do it fast and it would take me longer to study a new fork and merge my changes into this one than recreate 3 types.

aarani commented 4 years ago

TLRequestEditBanned 0x72796912 TLChatBannedRights -1626209256

that's the problem if you update those you have to update every type they reference too (like channel in your situation). and if you use layer 99 for example to connect to telegram servers and use another old method it will not recognize those method and you have to update all of them to the new layer too. UPDATING CONSTRUCTORS Only IS NOT ENOUGH EITHER

of course, i also recreated the types.

Please don't do this manually, use Afshin's PR, fork from his branch.

i had to do it fast and it would take me longer to study a new fork and merge my changes into this one than recreate 3 types.

It's way faster to study the new fork to update all types you need to send or gonna receive from telegram servers. there is a reason that we have a code generator for that

solarin commented 4 years ago

TLRequestEditBanned 0x72796912 TLChatBannedRights -1626209256

that's the problem if you update those you have to update every type they reference too (like channel in your situation). and if you use layer 99 for example to connect to telegram servers and use another old method it will not recognize those method and you have to update all of them to the new layer too. UPDATING CONSTRUCTORS Only IS NOT ENOUGH EITHER

i see! and where do i find which which layer one type belongs to?

for example, if I see https://core.telegram.org/method/channels.editBanned

It shows layer 108 on top right. however this type 0x‭4DF30834‬ doesn't show up there, so, how do i know what types are implemented in one layer?

aarani commented 4 years ago

TLRequestEditBanned 0x72796912 TLChatBannedRights -1626209256

that's the problem if you update those you have to update every type they reference too (like channel in your situation). and if you use layer 99 for example to connect to telegram servers and use another old method it will not recognize those method and you have to update all of them to the new layer too. UPDATING CONSTRUCTORS Only IS NOT ENOUGH EITHER

i see! and where do i find which which layer one type belongs to?

for example, if I see https://core.telegram.org/method/channels.editBanned

It shows layer 108 on top right. however this type 0x‭4DF30834‬ doesn't show up there, so, how do i know what types are implemented in one layer?

just specify Layer 108 in TelegramClient.cs inside the InitConenction Call and update every types to layer 108

solarin commented 4 years ago

ok then

thank you :)

solarin commented 4 years ago

ok, last question, which layer is supported by this branch? and where can i see the supported methods by this layer?

thanks for your patience

solarin commented 4 years ago

layer 66, i just saw, which is not on the layers listed on telgram.org... :(((

knocte commented 4 years ago

which layer is supported by this branch?

define "this branch"? If you mean master, 66. If you mean Afshin's PR, 108. I already said this above ;)

solarin commented 4 years ago

which layer is supported by this branch?

define "this branch"? If you mean master, 66. If you mean Afshin's PR, 108. I already said this above ;)

yes, i referred to the master.

i'll have to update to 108 or it's gonna be difficult

knocte commented 4 years ago

which is not on the layers listed on telgram.org... :(((

Why sad? You know that Afshin's branch is layer 108, the earlier you try it (and thus receive feedback from someone), the earlier we will merge it.

solarin commented 4 years ago

which is not on the layers listed on telgram.org... :(((

Why sad? You know that Afshin's branch is layer 108, the earlier you try it (and thus receive feedback from someone), the earlier we will merge it.

because i can't find any reference for layer 66

knocte commented 4 years ago

i'll have to update to 108 or it's gonna be difficult

why difficult? git remote add and git pull are difficult commands? I don't understand your fear, really.

knocte commented 4 years ago

because i can't find any reference for layer 66

Why do you care about that layer? it's very old, that's why we're working on the layer update to the latest (108)

solarin commented 4 years ago

ok let me see

solarin commented 4 years ago

because i can't find any reference for layer 66

Why do you care about that layer? it's very old, that's why we're working on the layer update to the latest (108)

because i have written a lot of code for my project which i'll have to merge and test

knocte commented 4 years ago

because i have written a lot of code for my project which i'll have to merge and test

I think you're overestimating the porting effort, because as far as I understand, layer updates bring new features, so the changes over already existing features should be very minimal, if not inexistent.

knocte commented 4 years ago

I guess you don't have this problem anymore after using layer108 right? Can I close this?

solarin commented 4 years ago

yes