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

Should remove the panic? #108

Open
waybackarchiver opened this issue Aug 6, 2021 · 3 comments
Open

Should remove the panic? #108

waybackarchiver opened this issue Aug 6, 2021 · 3 comments

Comments

@waybackarchiver
Copy link
Contributor

The panic in this code looks like used for debugging. I guess that should remove it?

mtproto/mtproto.go

Lines 364 to 367 in 04ee89d

case *objects.BadMsgNotification:
pp.Println(message)
panic(message) // for debug, looks like this message is important
return BadMsgErrorFromNative(message)

@quenbyako
Copy link
Member

@waybackarchiver well, yes, but no. BadMsgNotification means that all session was broken, i mean, this message should be never received from official telegram servers. if we got it, then something in package is broken.

Soooo, idk, maybe we need to leave for now... Or remove it, cause i hate panics in community modules too...

I don't know, need to decide what to do with it.

@waybackarchiver
Copy link
Contributor Author

As I understand it, we should provide this panic as an error to the caller to handling, and not have them interrupted.

But I haven't encountered this yet, so maybe I'll consider it later if we need to.

@sunnyque
Copy link

return error (or mark session is in bad state) and let app decide what to do (panic or reconnect/recreate session). at this time I've been forced to separate app logic using different processes because of mtproto panics (lost connection/BadMsgNotification whatever)

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

3 participants