gotd / td

Telegram client, in Go. (MTProto API)
MIT License
1.49k stars 134 forks source link

Return 401 unauthorized errors #1458

Open prdsrm opened 2 days ago

prdsrm commented 2 days ago

Description: status of the proposed feature

Currently, when using gotd with long running process, when a session gets disconnected, for example with AUTH_KEY_UNREGISTERED, or SESSION_EXPIRED/REVOKED error, the client.Run function does not return an error, it only logs it. This means that the client ends up looping indefinitely, logging an error: Got error on self.... AUTH_KEY_UNREGISTERED|SESSION_EXPIRED.

And this is hard to catch because if you do not use the provided logger(a *zap.Logger instance), you won't be able to understand what is happening.

As you can see here, in the telegram/connect.go:

        g.Go(func(ctx context.Context) error {
            // Call method which requires authorization, to subscribe for updates.
            // See https://core.telegram.org/api/updates#subscribing-to-updates.
            self, err := c.Self(ctx)
            if err != nil {
                // Ignore unauthorized errors.
                if !auth.IsUnauthorized(err) {
                                         // The error is logged, not returned
                    c.log.Warn("Got error on self", zap.Error(err))
                }
                                 // Nothing is returned
                return nil
            }

            c.log.Info("Got self", zap.String("username", self.Username))
            return nil
        })

Here is an example code where the client tends to loop indefinitely:

    if err := client.Run(ctx, func(ctx context.Context) error {
        authCli := client.Auth()
        // Checking auth status.
        status, err := authCli.Status(ctx)
        if err != nil {
            return err
        }
        // Can be already authenticated if we have valid session in
        // session storage.
        if !status.Authorized {
            if err := client.Auth().IfNecessary(ctx, flow); err != nil {
                return fmt.Errorf("could not authenticate: %w", err)
            }
        }
        if err := f(ctx, client, dispatcher, options); err != nil {
            return err
        }
        return nil
    }); err != nil {
        return err
    }

Description: possible solution

The solution would be to detect those 401 errors, and return them immediately, so the user of the library can handle them properly. Or at least, make it more explicit for people not using the standard *zap.Logger that something is not working right now. Or, provide an example that shows how to check for those errors. Like checking if the session is currently still working, and no AUTH_KEY_UNREGISTERED or SESSION_EXPIRED errors are occuring.

So, I suppose doing something similar:

            // Call method which requires authorization, to subscribe for updates.
            // See https://core.telegram.org/api/updates#subscribing-to-updates.
            self, err := c.Self(ctx)
            if err != nil {
                // Ignore unauthorized errors.
                if !auth.IsUnauthorized(err) {
                    c.log.Warn("Got error on self", zap.Error(err))
                }
                return err
                         }

Current workaround

In order to detect it, what I'm doing right now is to set a timeout with time.After, while running the client.Run function in a goroutine on a function that is expected to take less than 10 seconds. The timeout is like 30 seconds, and it allows me to detect that a session is now unregistered / expired.

References

https://core.telegram.org/api/errors#401-unauthorized

Funding

So, I set a small 20$ funding depending on the difficulty of the issue, I may slightly increase it if its hard to do. https://polar.sh/gotd/td/issues/1458

ernado commented 14 hours ago

Are there any logs near the Got error on self message? Something like Restarting connection?

We have following method:

func (c *Client) isPermanentError(err error) bool {
    return errors.Is(err, exchange.ErrKeyFingerprintNotFound)
}

That is used in reconnectUntilClosed method of client:

https://github.com/gotd/td/blob/244876abc442f57822cccbe942e37c7a980cf74f/telegram/connect.go#L60-L81

If true, we can change isPermanentError to detect AUTH_KEY_UNREGISTERED and SESSION_EXPIRED as permanent errors and this should fix problem described in the issue.