connector: fetch @me to create login before creating client

Creating the client before the actual UserLogin is bad form.

pkg/connector/client.go

@@ -54,7 +54,6 @@ func (d *DiscordConnector) LoadUserLogin(ctx context.Context, login *bridgev2.Us
 	meta := login.Metadata.(*discordid.UserLoginMetadata)
 
 	session, err := NewDiscordSession(ctx, meta.Token)
-	login.Save(ctx)
 
 	if err != nil {
 		return err
@@ -65,7 +64,6 @@ func (d *DiscordConnector) LoadUserLogin(ctx context.Context, login *bridgev2.Us
 		UserLogin: login,
 		Session:   session,
 	}
-	cl.SetUp(ctx, meta)
 
 	login.Client = &cl
 
@@ -74,46 +72,31 @@ func (d *DiscordConnector) LoadUserLogin(ctx context.Context, login *bridgev2.Us
 
 var _ bridgev2.NetworkAPI = (*DiscordClient)(nil)
 
-// SetUp performs basic bookkeeping and initialization that should be done
-// immediately after a DiscordClient has been created.
-//
-// nil may be passed for meta, especially during provisioning where we need to
-// connect to the Discord gateway, but don't have a UserLogin yet.
-func (d *DiscordClient) SetUp(ctx context.Context, meta *discordid.UserLoginMetadata) {
-	// TODO: Turn this into a factory function like `NewDiscordClient`.
-	log := zerolog.Ctx(ctx)
-
-	// We'll have UserLogin metadata if this UserLogin is being loaded from the
-	// database, i.e. it hasn't just been provisioned.
-	if meta != nil {
-		if meta.HeartbeatSession.IsExpired() {
-			log.Info().Msg("Heartbeat session expired, creating a new one")
-			meta.HeartbeatSession = discordgo.NewHeartbeatSession()
-		}
-		meta.HeartbeatSession.BumpLastUsed()
-		d.Session.HeartbeatSession = meta.HeartbeatSession
-	}
-
-	d.markedOpened = make(map[string]time.Time)
-}
-
 func (d *DiscordClient) Connect(ctx context.Context) {
 	log := zerolog.Ctx(ctx)
 
-	if d.Session == nil {
-		log.Error().Msg("No session present")
-		d.UserLogin.BridgeState.Send(status.BridgeState{
-			StateEvent: status.StateBadCredentials,
-			Error:      "discord-not-logged-in",
-		})
-		return
-	}
+	meta := d.UserLogin.Metadata.(*discordid.UserLoginMetadata)
 
+	if meta.HeartbeatSession.IsExpired() {
+		log.Info().Msg("Heartbeat session expired, creating a new one")
+		meta.HeartbeatSession = discordgo.NewHeartbeatSession()
+	}
+	meta.HeartbeatSession.BumpLastUsed()
+	d.Session.HeartbeatSession = meta.HeartbeatSession
+
+	d.markedOpened = make(map[string]time.Time)
+
+	log.Debug().Msg("Connecting to Discord")
 	d.UserLogin.BridgeState.Send(status.BridgeState{
 		StateEvent: status.StateConnecting,
 	})
 	if err := d.connect(ctx); err != nil {
 		log.Err(err).Msg("Couldn't connect to Discord")
+		d.UserLogin.BridgeState.Send(status.BridgeState{
+			StateEvent: status.StateUnknownError,
+			Error:      "discord-connect-error",
+			Message:    err.Error(),
+		})
 	}
 }
 
@@ -152,11 +135,7 @@ func (cl *DiscordClient) connect(ctx context.Context) error {
 		cl.usersFromReady[user.ID] = user
 	}
 
-	// NOTE: We won't have a UserLogin during provisioning, because the UserLogin
-	// can only be properly constructed once we know what the Discord user ID is
-	// (i.e. we have returned from this function). We'll rely on the login
-	// process calling this method manually instead.
-	cl.BeginSyncingIfUserLoginPresent(ctx)
+	cl.BeginSyncing(ctx)
 
 	return nil
 }
@@ -175,11 +154,7 @@ func (d *DiscordClient) LogoutRemote(ctx context.Context) {
 	d.Disconnect()
 }
 
-func (cl *DiscordClient) BeginSyncingIfUserLoginPresent(ctx context.Context) {
-	if cl.UserLogin == nil {
-		cl.connector.Bridge.Log.Warn().Msg("Not syncing just yet as we don't have a UserLogin")
-		return
-	}
+func (cl *DiscordClient) BeginSyncing(ctx context.Context) {
 	if cl.hasBegunSyncing {
 		cl.connector.Bridge.Log.Warn().Msg("Not beginning sync more than once")
 		return

pkg/connector/handlediscord.go

@@ -164,23 +164,6 @@ func (d *DiscordClient) wrapDiscordReaction(reaction *discordgo.MessageReaction,
 }
 
 func (d *DiscordClient) handleDiscordEvent(rawEvt any) {
-	if d.UserLogin == nil {
-		// Our event handlers are able to assume that a UserLogin is available.
-		// We respond to special events like READY outside of this function,
-		// by virtue of methods like Session.Open only returning control flow
-		// after RESUME or READY.
-		log := zerolog.Ctx(context.TODO())
-		log.Trace().Msg("Dropping Discord event received before UserLogin creation")
-		return
-	}
-
-	if d.Session == nil || d.Session.State == nil || d.Session.State.User == nil {
-		// Our event handlers are able to assume that we've fully connected to the
-		// gateway.
-		d.UserLogin.Log.Debug().Msg("Dropping Discord event received before READY or RESUMED")
-		return
-	}
-
 	defer func() {
 		err := recover()
 		if err != nil {

pkg/connector/login_generic.go

@@ -47,55 +47,47 @@ type DiscordGenericLogin struct {
 }
 
 func (dl *DiscordGenericLogin) FinalizeCreatingLogin(ctx context.Context, token string) (*bridgev2.UserLogin, error) {
+	log := zerolog.Ctx(ctx).With().Str("action", "finalize login").Logger()
+
+	log.Info().Msg("Creating session with provided token")
 	session, err := NewDiscordSession(ctx, token)
 	if err != nil {
 		return nil, fmt.Errorf("couldn't create discord session: %w", err)
 	}
-
-	client := DiscordClient{
-		connector: dl.connector,
-		Session:   session,
-	}
-	client.SetUp(ctx, nil)
-
-	err = client.connect(ctx)
-	if err != nil {
-		dl.Cancel()
-		return nil, err
-	}
-	// At this point we've opened a WebSocket connection to the gateway, received
-	// a READY packet, and know who we are.
-	user := session.State.User
-	dl.DiscordUser = user
-
 	dl.Session = session
+
+	log.Info().Msg("Requesting @me with provided token")
+	self, err := session.User("@me")
+	if err != nil {
+		return nil, fmt.Errorf("couldn't request self user (bad credentials?): %w", err)
+	}
+	dl.DiscordUser = self
+
+	log.Info().Msg("Fetched @me")
 	ul, err := dl.User.NewLogin(ctx, &database.UserLogin{
-		ID: discordid.MakeUserLoginID(user.ID),
+		ID: discordid.MakeUserLoginID(self.ID),
 		Metadata: &discordid.UserLoginMetadata{
 			Token:            token,
 			HeartbeatSession: session.HeartbeatSession,
 		},
 	}, &bridgev2.NewLoginParams{
 		LoadUserLogin: func(ctx context.Context, login *bridgev2.UserLogin) error {
-			login.Client = &client
-			client.UserLogin = login
-
-			// Only now that we have a UserLogin can we begin syncing.
-			client.BeginSyncingIfUserLoginPresent(ctx)
+			// (mautrix will instead call `LoadUserLogin` on the connector if we don't provide this.)
+			login.Client = &DiscordClient{
+				connector: dl.connector,
+				UserLogin: login,
+				Session:   session,
+			}
 			return nil
 		},
-		DeleteOnConflict:  true,
-		DontReuseExisting: false,
+		DeleteOnConflict: true,
 	})
 	if err != nil {
 		dl.Cancel()
-		return nil, fmt.Errorf("couldn't create login: %w", err)
+		return nil, fmt.Errorf("couldn't create login during finalization: %w", err)
 	}
 
-	zerolog.Ctx(ctx).Info().
-		Str("user_id", user.ID).
-		Str("user_username", user.Username).
-		Msg("Logged in to Discord")
+	(ul.Client.(*DiscordClient)).Connect(ctx)
 
 	return ul, nil
 }