Fix or ignore some known errors

ref DEV-2376
authgear · Jan 9, 2025 · a9e0b1c · a9e0b1c
2 parents ba2ab31 + 4b31dfd
commit a9e0b1c
Show file tree

Hide file tree

Showing 12 changed files with 62 additions and 34 deletions.
diff --git a/pkg/auth/handler/webapp/authflow_forgot_password.go b/pkg/auth/handler/webapp/authflow_forgot_password.go
@@ -9,8 +9,8 @@ import (
 	"github.com/authgear/authgear-server/pkg/auth/webapp"
 	authflow "github.com/authgear/authgear-server/pkg/lib/authenticationflow"
 	"github.com/authgear/authgear-server/pkg/lib/authenticationflow/declarative"
-	"github.com/authgear/authgear-server/pkg/lib/authn/otp"
 	"github.com/authgear/authgear-server/pkg/lib/config"
+	"github.com/authgear/authgear-server/pkg/lib/infra/whatsapp"
 	"github.com/authgear/authgear-server/pkg/util/httproute"
 	"github.com/authgear/authgear-server/pkg/util/template"
 	"github.com/authgear/authgear-server/pkg/util/validation"
@@ -194,7 +194,7 @@ func (h *AuthflowForgotPasswordHandler) ServeHTTP(w http.ResponseWriter, r *http
 		inputs := h.makeInputs(screen, identification, loginID, 0)
 
 		result, err := h.Controller.AdvanceWithInputs(ctx, r, s, screen, inputs, nil)
-		if errors.Is(err, otp.ErrInvalidWhatsappUser) {
+		if errors.Is(err, whatsapp.ErrInvalidWhatsappUser) {
 			// The code failed to send because it is not a valid whatsapp user
 			// Try again with sms if possible
 			var fallbackErr error

diff --git a/pkg/auth/handler/webapp/authflowv2/forgot_password.go b/pkg/auth/handler/webapp/authflowv2/forgot_password.go
@@ -12,8 +12,8 @@ import (
 	"github.com/authgear/authgear-server/pkg/auth/webapp"
 	authflow "github.com/authgear/authgear-server/pkg/lib/authenticationflow"
 	"github.com/authgear/authgear-server/pkg/lib/authenticationflow/declarative"
-	"github.com/authgear/authgear-server/pkg/lib/authn/otp"
 	"github.com/authgear/authgear-server/pkg/lib/config"
+	"github.com/authgear/authgear-server/pkg/lib/infra/whatsapp"
 	"github.com/authgear/authgear-server/pkg/util/httproute"
 	"github.com/authgear/authgear-server/pkg/util/template"
 	"github.com/authgear/authgear-server/pkg/util/validation"
@@ -265,7 +265,7 @@ func (h *AuthflowV2ForgotPasswordHandler) ServeHTTP(w http.ResponseWriter, r *ht
 		}
 
 		result, err := h.Controller.AdvanceWithInputs(ctx, r, s, screen, inputs, nil)
-		if errors.Is(err, otp.ErrInvalidWhatsappUser) {
+		if errors.Is(err, whatsapp.ErrInvalidWhatsappUser) {
 			// The code failed to send because it is not a valid whatsapp user
 			// Try again with sms if possible
 			var fallbackErr error

diff --git a/pkg/auth/webapp/session_authflow.go b/pkg/auth/webapp/session_authflow.go
@@ -11,8 +11,8 @@ import (
 	"github.com/authgear/authgear-server/pkg/api/model"
 	authflow "github.com/authgear/authgear-server/pkg/lib/authenticationflow"
 	"github.com/authgear/authgear-server/pkg/lib/authenticationflow/declarative"
-	"github.com/authgear/authgear-server/pkg/lib/authn/otp"
 	"github.com/authgear/authgear-server/pkg/lib/config"
+	"github.com/authgear/authgear-server/pkg/lib/infra/whatsapp"
 	"github.com/authgear/authgear-server/pkg/util/base32"
 	corerand "github.com/authgear/authgear-server/pkg/util/rand"
 )
@@ -775,7 +775,7 @@ func (s *AuthflowScreenWithFlowResponse) makeFallbackToSMSFromWhatsappRetryHandl
 		if disableFallbackToSMS {
 			return nil
 		}
-		if !errors.Is(err, otp.ErrInvalidWhatsappUser) {
+		if !errors.Is(err, whatsapp.ErrInvalidWhatsappUser) {
 			return nil
 		}
 		smsChannelIdx := -1

diff --git a/pkg/lib/authn/otp/errors.go b/pkg/lib/authn/otp/errors.go
@@ -6,16 +6,11 @@ import (
 )
 
 var InvalidOTPCode = apierrors.Forbidden.WithReason("InvalidOTPCode")
-var InvalidWhatsappUser = apierrors.BadRequest.WithReason("InvalidWhatsappUser")
-var NoAvailableWhatsappClient = apierrors.BadRequest.WithReason("NoAvailableWhatsappClient")
 
 var ErrCodeNotFound = InvalidOTPCode.NewWithCause("otp code is expired or invalid", apierrors.StringCause("CodeNotFound"))
 var ErrInvalidCode = InvalidOTPCode.NewWithCause("invalid otp code", apierrors.StringCause("InvalidCode"))
 var ErrConsumedCode = InvalidOTPCode.NewWithCause("used otp code", apierrors.StringCause("UsedCode"))
 
-var ErrInvalidWhatsappUser = InvalidWhatsappUser.New("invalid whatsapp user")
-var ErrNoAvailableWhatsappClient = NoAvailableWhatsappClient.New("no available whatsapp client")
-
 // FIXME: backward compat; should not use RateLimited
 var ErrTooManyAttempts = ratelimit.RateLimited.NewWithInfo("too many verify OTP attempts", apierrors.Details{
 	"bucket_name": "TrackFailedOTPAttemptBucket",

diff --git a/pkg/lib/authn/otp/sender.go b/pkg/lib/authn/otp/sender.go
@@ -2,7 +2,6 @@ package otp
 
 import (
 	"context"
-	"errors"
 	neturl "net/url"
 	"path/filepath"
 
@@ -220,15 +219,6 @@ func (s *MessageSender) sendSMS(ctx context.Context, opts SendOptions) error {
 }
 
 func (s *MessageSender) sendWhatsapp(ctx context.Context, opts SendOptions) (err error) {
-	defer func() {
-		if err != nil {
-			if errors.Is(err, whatsapp.ErrInvalidUser) {
-				err = ErrInvalidWhatsappUser
-			} else if errors.Is(err, whatsapp.ErrNoAvailableClient) {
-				err = ErrNoAvailableWhatsappClient
-			}
-		}
-	}()
 
 	spec := s.selectMessage(opts.Form, opts.Type)
 	msgType := spec.MessageType

diff --git a/pkg/lib/infra/whatsapp/errors.go b/pkg/lib/infra/whatsapp/errors.go
@@ -1,9 +1,18 @@
 package whatsapp
 
-import "errors"
+import (
+	"errors"
+
+	"github.com/authgear/authgear-server/pkg/api/apierrors"
+)
+
+var ErrInvalidWhatsappUser = apierrors.BadRequest.
+	WithReason("InvalidWhatsappUser").
+	New("invalid whatsapp user")
+var ErrNoAvailableWhatsappClient = apierrors.BadRequest.
+	WithReason("NoAvailableWhatsappClient").
+	New("no available whatsapp client")
 
 var ErrUnauthorized = errors.New("whatsapp: unauthorized")
-var ErrInvalidUser = errors.New("whatsapp: invalid user")
 var ErrBadRequest = errors.New("whatsapp: bad request")
 var ErrUnexpectedLoginResponse = errors.New("whatsapp: unexpected login response body")
-var ErrNoAvailableClient = errors.New("whatsapp: no available client")
diff --git a/pkg/lib/infra/whatsapp/on_premises.go b/pkg/lib/infra/whatsapp/on_premises.go
@@ -163,7 +163,7 @@ func (c *OnPremisesClient) parseBadRequestError(resp *http.Response) error {
 		if errResp.Errors != nil && len(*errResp.Errors) > 0 {
 			switch (*errResp.Errors)[0].Code {
 			case 1013:
-				return ErrInvalidUser
+				return ErrInvalidWhatsappUser
 			}
 		}
 	}

diff --git a/pkg/lib/infra/whatsapp/service.go b/pkg/lib/infra/whatsapp/service.go
@@ -74,7 +74,7 @@ func (s *Service) ResolveSendAuthenticationOTPOptions(ctx context.Context, opts
 	switch s.WhatsappConfig.APIType {
 	case config.WhatsappAPITypeOnPremises:
 		if s.OnPremisesClient == nil {
-			return nil, ErrNoAvailableClient
+			return nil, ErrNoAvailableWhatsappClient
 		}
 
 		otpTemplate := s.OnPremisesClient.GetOTPTemplate()
@@ -98,7 +98,7 @@ func (s *Service) SendAuthenticationOTP(ctx context.Context, opts *ResolvedSendA
 	switch s.WhatsappConfig.APIType {
 	case config.WhatsappAPITypeOnPremises:
 		if s.OnPremisesClient == nil {
-			return ErrNoAvailableClient
+			return ErrNoAvailableWhatsappClient
 		}
 
 		return s.OnPremisesClient.SendTemplate(

diff --git a/pkg/lib/messaging/sender.go b/pkg/lib/messaging/sender.go
@@ -199,6 +199,7 @@ func (s *Sender) SendSMSInNewGoroutine(ctx context.Context, msgType translation.
 				otelauthgear.WithStatusError(),
 			)
 
+			// TODO: Handle expected errors https://linear.app/authgear/issue/DEV-1139
 			s.Logger.WithError(err).WithFields(logrus.Fields{
 				"phone": phone.Mask(opts.To),
 			}).Error("failed to send SMS")
@@ -297,15 +298,26 @@ func (s *Sender) SendWhatsappImmediately(ctx context.Context, msgType translatio
 	// Send immediately.
 	err = s.sendWhatsapp(ctx, opts)
 	if err != nil {
+		isInvalidWhatsappUserErr := errors.Is(err, whatsapp.ErrInvalidWhatsappUser)
+		status := otelauthgear.WithStatusError()
+		if isInvalidWhatsappUserErr {
+			status = otelauthgear.WithStatusInvalidWhatsappUser()
+		}
 		otelauthgear.IntCounterAddOne(
 			ctx,
 			otelauthgear.CounterWhatsappRequestCount,
-			otelauthgear.WithStatusError(),
+			status,
 		)
 
-		s.Logger.WithError(err).WithFields(logrus.Fields{
-			"phone": phone.Mask(opts.To),
-		}).Error("failed to send Whatsapp")
+		if !isInvalidWhatsappUserErr {
+			s.Logger.WithError(err).WithFields(logrus.Fields{
+				"phone": phone.Mask(opts.To),
+			}).Error("failed to send Whatsapp")
+		} else {
+			s.Logger.WithError(err).WithFields(logrus.Fields{
+				"phone": phone.Mask(opts.To),
+			}).Warn("failed to send Whatsapp")
+		}
 
 		logErr := s.Events.DispatchEventImmediately(ctx, &nonblocking.WhatsappErrorEventPayload{
 			Description: s.errorToDescription(err),

diff --git a/pkg/lib/oauth/handler/handler_token.go b/pkg/lib/oauth/handler/handler_token.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"reflect"
 	"strings"
 	"time"
 
@@ -1062,7 +1063,13 @@ func (h *TokenHandler) handleBiometricSetup(
 			JWT: r.JWT(),
 		})
 		if len(edges) != 0 {
-			return nil, errors.New("interaction no completed for biometric setup")
+			h.Logger.WithFields(map[string]interface{}{
+				"cliend_id": client.ClientID,
+				"edges": strings.Join(slice.Map(edges, func(edge interaction.Edge) string {
+					return reflect.TypeOf(edge).String()
+				}), ","),
+			}).Error("interaction not completed for biometric setup")
+			return nil, errors.New("interaction not completed for biometric setup")
 		} else if err != nil {
 			return nil, err
 		}
@@ -1119,7 +1126,13 @@ func (h *TokenHandler) handleBiometricAuthenticate(
 			JWT: r.JWT(),
 		})
 		if len(edges) != 0 {
-			return nil, errors.New("interaction no completed for biometric authenticate")
+			h.Logger.WithFields(map[string]interface{}{
+				"cliend_id": client.ClientID,
+				"edges": strings.Join(slice.Map(edges, func(edge interaction.Edge) string {
+					return reflect.TypeOf(edge).String()
+				}), ","),
+			}).Error("interaction not completed for biometric authenticate")
+			return nil, errors.New("interaction not completed for biometric authenticate")
 		} else if err != nil {
 			return nil, err
 		}

diff --git a/pkg/lib/otelauthgear/metric.go b/pkg/lib/otelauthgear/metric.go
@@ -76,6 +76,9 @@ var AttributeStatusOK = AttributeKeyStatus.String("ok")
 // AttributeStatusError is "status=error".
 var AttributeStatusError = AttributeKeyStatus.String("error")
 
+// AttributeStatusInvalidWhatsappUser is "status=invalid_whatsapp_user".
+var AttributeStatusInvalidWhatsappUser = AttributeKeyStatus.String("invalid_whatsapp_user")
+
 var CounterOAuthSessionCreationCount = mustInt64Counter(
 	"authgear.oauth_session.creation.count",
 	metric.WithDescription("The number of creation of OAuth session"),
@@ -182,6 +185,10 @@ func WithStatusError() MetricOption {
 	return metricOptionAttributeKeyValue{AttributeStatusError}
 }
 
+func WithStatusInvalidWhatsappUser() MetricOption {
+	return metricOptionAttributeKeyValue{AttributeStatusInvalidWhatsappUser}
+}
+
 // IntCounterAddOne prepares necessary attributes and calls Add with incr=1.
 // It is intentionally that this does not accept metric.AddOption.
 // If this accepts metric.AddOption, then you can pass in arbitrary metric.WithAttributes.

diff --git a/pkg/lib/session/idpsession/provider.go b/pkg/lib/session/idpsession/provider.go
@@ -188,7 +188,9 @@ func (p *Provider) accessWithID(ctx context.Context, id string, accessEvent acce
 
 	s.AccessInfo.LastAccess = accessEvent
 	defer func() {
-		err = p.accessSideEffects(ctx, s, accessEvent)
+		if err == nil && s != nil {
+			err = p.accessSideEffects(ctx, s, accessEvent)
+		}
 	}()
 
 	setSessionExpireAtForResolvedSession(s, p.Config)