Return 401 unauthorized errors

[prdsrm]

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]

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:

td/telegram/connect.go

Lines 60 to 81 in 244876a

	func (c *Client) reconnectUntilClosed(ctx context.Context) error {
	// Note that we currently have no timeout on connection, so this is
	// potentially eternal.
	b := tdsync.SyncBackoff(backoff.WithContext(c.connBackoff(), ctx))
	return backoff.RetryNotify(func() error {
	if err := c.runUntilRestart(ctx); err != nil {
	if c.isPermanentError(err) {
	return backoff.Permanent(err)
	}
	return err
	}
	return nil
	}, b, func(err error, timeout time.Duration) {
	c.log.Info("Restarting connection", zap.Error(err), zap.Duration("backoff", timeout))
	c.connMux.Lock()
	c.conn = c.createPrimaryConn(nil)
	c.connMux.Unlock()
	})
	}

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.

[ernado]

Can you please try https://github.com/gotd/td/releases/tag/v0.114.0-alpha.0?

[ernado]

The v0.114.0-alpha.1 is available.
If you are using updates.Manager, this will happen automatically.

If not, you can do following:

for {
	select {
	case <-ctx.Done():
		return ctx.Err()
	case <-time.After(time.Second * 10):
		s, err := client.Self(ctx)
		if auth.IsUnauthorized(err) {
			return errors.Wrap(err, "got unauthorized")
		}
		fmt.Println("Current user:", s.FirstName)
	}
}

I've checked this on testserver.

Update handling disabled:

Current user: Foo
Current user: Foo
Error: run:
    main.run.func4
        /src/ernado/tg-reconnect-test/main.go:265
  - callback:
    github.com/gotd/td/telegram.(*Client).Run.func3
        /src/gotd/td/telegram/connect.go:168
  - got unauthorized:
    main.run.func4.1
        /src/ernado/tg-reconnect-test/main.go:252
  - rpcDoRequest:
    github.com/gotd/td/mtproto.(*Conn).Invoke
        /src/gotd/td/mtproto/rpc.go:44
  - rpc error code 401: AUTH_KEY_UNREGISTERED

Update handling enabled:

Error: run:
    main.run.func4
        /src/ernado/tg-reconnect-test/main.go:249
  - callback:
    github.com/gotd/td/telegram.(*Client).Run.func3
        /src/gotd/td/telegram/connect.go:168
  - fatal error:
    github.com/gotd/td/telegram/updates.(*internalState).Run
        /src/gotd/td/telegram/updates/state.go:181
  - get difference:
    github.com/gotd/td/telegram/updates.(*internalState).getDifference
        /src/gotd/td/telegram/updates/state.go:404
  - rpcDoRequest:
    github.com/gotd/td/mtproto.(*Conn).Invoke
        /src/gotd/td/mtproto/rpc.go:44
  - rpc error code 401: AUTH_KEY_UNREGISTERED

Same logic should apply to reconnectUntilClosed so we don't reconnect eternally if key is unregistered.

Please ping me here if issue is not resolved, I will reopen.

[ernado]

Also you can use new OnSelfError in https://pkg.go.dev/github.com/gotd/[email protected]/telegram#Options
but please note that error on first connect (i.e. before auth) is expected.

[prdsrm]

Also you can use new OnSelfError in https://pkg.go.dev/github.com/gotd/[email protected]/telegram#Options but please note that error on first connect (i.e. before auth) is expected.

Thanks! I will test this later.

So OnSelfError can be used for errors like AUTH_KEY_DUPLICATED, that are not handled by the added code, which only check for AUTH_KEY_UNREGISTERED & SESSION_EXPIRED? That's great

	// See https://github.com/gotd/td/issues/1458.
	if errors.Is(err, exchange.ErrKeyFingerprintNotFound) {
		return true
	}
	if tgerr.Is(err, "AUTH_KEY_UNREGISTERED") || tgerr.Is(err, "SESSION_EXPIRED") {
		return true
	}
	if auth.IsUnauthorized(err) {
		return true
	}
	return false

[ernado]

Yes, you can check for any error, the #1478 handling can be pretty flexible.
Hopefully it will help.

[ernado]

BTW I think that AUTH_KEY_DUPLICATED should have 401 code too and this code should handle it.
Anyway I've also added AUTH_KEY_DUPLICATED error to

if tgerr.Is(err, "AUTH_KEY_UNREGISTERED", "SESSION_EXPIRED", "AUTH_KEY_DUPLICATED") {
	return true
}

list, thank you.