Skip to content

Commit 278aa80

Browse files
committed
fix: move email & sms send out of the POST /user transaction
1 parent 269ddfe commit 278aa80

File tree

1 file changed

+20
-8
lines changed

1 file changed

+20
-8
lines changed

internal/api/user.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
181181
}
182182
}
183183

184+
var sendEmailChange, sendPhoneConfirmation bool
185+
flowType := getFlowFromChallenge(params.CodeChallenge)
186+
184187
err := db.Transaction(func(tx *storage.Connection) error {
185188
var terr error
186189
if params.Password != nil {
@@ -223,17 +226,14 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
223226
}
224227

225228
} else {
226-
flowType := getFlowFromChallenge(params.CodeChallenge)
227229
if isPKCEFlow(flowType) {
228230
_, terr := generateFlowState(tx, models.EmailChange.String(), models.EmailChange, params.CodeChallengeMethod, params.CodeChallenge, &user.ID)
229231
if terr != nil {
230232
return terr
231233
}
232-
233-
}
234-
if terr = a.sendEmailChange(r, tx, user, params.Email, flowType); terr != nil {
235-
return terr
236234
}
235+
236+
sendEmailChange = true
237237
}
238238
}
239239

@@ -247,9 +247,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
247247
return terr
248248
}
249249
} else {
250-
if _, terr := a.sendPhoneConfirmation(r, tx, user, params.Phone, phoneChangeVerification, params.Channel); terr != nil {
251-
return terr
252-
}
250+
sendPhoneConfirmation = true
253251
}
254252
}
255253

@@ -263,5 +261,19 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
263261
return err
264262
}
265263

264+
if sendEmailChange {
265+
// email sending should not hold a database transaction open as latency incurred by SMTP or HTTP hooks can exhaust the database pool
266+
if err := a.sendEmailChange(r, db, user, params.Email, flowType); err != nil {
267+
return err
268+
}
269+
}
270+
271+
if sendPhoneConfirmation {
272+
// SMS sending should not hold a database transaction open as latency incurred by SMTP or HTTP hooks can exhaust the database pool
273+
if _, err := a.sendPhoneConfirmation(r, db, user, params.Phone, phoneChangeVerification, params.Channel); err != nil {
274+
return err
275+
}
276+
}
277+
266278
return sendJSON(w, http.StatusOK, user)
267279
}

0 commit comments

Comments
 (0)