ユーザーが自分の表示名を変更すると500エラーになる、という問い合わせを受けたことがあります。特定の会社に所属するユーザーでだけ起きていて、こちらの手元ではまったく再現しませんでした。
Sentryに上がっていたのはNoMethodErrorで、発生箇所は名前を更新する処理そのものではなく、更新後にWebSocketで他のメンバーへ通知を飛ばすコールバックの中でした。それを見た時点で私は「通知が失敗しているだけなら、名前の変更自体は保存されているはずだ」と考えたのですが、これが完全に間違いでした。実際にはユーザー名のUPDATEごと、まるごとロールバックされていました。
以下のコード例は実際のプロダクトコードそのものではなく、同じ構造が伝わるように書き起こしたサンプルです。
何が起きていたか
問題のモデルは、こういう作りになっていました。
class User < ApplicationRecord
belongs_to :company
after_update :send_update_event_to_websockets
private
def send_update_event_to_websockets
WebsocketNotifier.notify(company.websocket_event_data)
end
end
Company#websocket_event_dataは、その会社に紐づくユーザーの情報を集めて配信用のハッシュを組み立てるメソッドでした。ところが、あるデータでは参照先のレコードがすでに存在しなくなっており、そこでnilに対してメソッドを呼んでしまいNoMethodErrorになっていました。
つまり、名前の変更というごく普通の更新が、そのユーザーとは直接関係のない「会社側の壊れたデータ」に巻き込まれて落ちていた、という構図です。
save/updateはコールバックまで含めて1つのトランザクション
ここで肝心なのが、saveやupdateの挙動です。Active Recordは、バリデーションからbefore/around/afterの各コールバックまでを、まとめて1つのトランザクションで包みます。
そして名前が紛らわしいのですが、after_saveやafter_updateは「コミット後」ではなくコミット直前に実行されます。実際に走る順序はこうなります。
- トランザクション開始（
BEGIN） UPDATE users SET name = ...-
after_update（ここで通知を飛ばしている） - コミット（
COMMIT）
3番目で例外が投げられると、当然4番目には到達しません。トランザクションはROLLBACKされ、2番目で成功していたはずのUPDATEもなかったことになります。Railsのドキュメントでも、saveとdestroyはトランザクションで包まれており、after_*を含むコールバック内で例外を投げればロールバックできる、と明記されています。
一方でafter_commitはコミットが確定した後に走るため、その中で例外が起きてもDBの変更は巻き戻りません。外部システムとのやり取りがafter_commit向きだと言われるのは、この違いがあるからですね。
「値は入っているのに500」という思い込みが危なかった
今回いちばん時間を溶かしたのは、技術的な難しさではなく、この思い込みでした。
「通知処理でコケているだけ」と決めつけていたので、当初はWebSocket側の配信基盤やコネクション周りを疑って調べていました。さらに厄介なことに、別の画面では変更後の名前が表示されているように見えるタイミングがあり、「DBは更新できているのにレスポンスだけエラーになっている」という仮説をしばらく捨てられませんでした。実際にはフロント側が楽観的に画面を書き換えていただけで、リロードすれば元の名前に戻ります。
結局、確実だったのはSQLログを見ることでした。
BEGIN
UPDATE "users" SET "name" = $1 ... WHERE "users"."id" = $2
ROLLBACK
COMMITではなくROLLBACKで閉じていることが分かった時点で、話がようやく前に進みました。表示や画面の挙動から「保存されているかどうか」を推測するのはやめて、ROLLBACKのログか、レコードをreloadして値を読み直すところまでやる。当たり前のことなのですが、原因を早合点しているときほど省略してしまいます。
対策として取れる選択肢
コミット確定後でよい処理は after_commit に移す
通知・外部API呼び出し・メール送信のように「保存が確定してから実行したい」処理は、素直にafter_commitへ移すのが基本です。
after_commit :send_update_event_to_websockets, on: :update
ただし、これだけでは「外部サービスの応答を待つ間リクエストがブロックされる」問題は解決しません。通知処理自体をジョブに逃がして非同期化するところまでセットで考える必要があります。
コールバック内で防御的にガードする
今回の直接の原因は、関連レコードがnilになりうる箇所を無防備に触っていたことでした。
def send_update_event_to_websockets
data = company&.websocket_event_data
return if data.blank?
WebsocketNotifier.notify(data)
end
対症療法ではありますが、「壊れたデータ1件のせいで、無関係なユーザーの更新が全部落ちる」という影響範囲の広がり方は確実に止められます。
保存と通知を呼び出し側で分ける
もう少し踏み込むなら、そもそもモデルのコールバックに通知を持たせない、という選択もあります。
ActiveRecord::Base.transaction do
user.update!(name: new_name)
end
begin
WebsocketNotifier.notify(user.company.websocket_event_data)
rescue => e
Sentry.capture_exception(e)
end
保存の成否と通知の成否を独立させられますし、「なぜ名前の更新でWebSocketのコードが動くのか」を追う手間もなくなります。単一責任という意味では、個人的にはこの形がいちばん好みです。
まとめ
-
save/updateはバリデーションと全コールバックを1つのトランザクションで包む -
after_save/after_updateはコミット前に走るので、そこでの例外は保存処理全体をロールバックさせる - 「副作用の処理だけ失敗して、本体の更新は成功している」という状態には、基本的にならない
- 保存されたかどうかは画面の見た目で判断せず、
ROLLBACKのログかreloadで裏を取る - コミット確定後でよい処理は
after_commitへ、あるいはコールバックから外に出す
コールバックは書くのは楽なのですが、「どのタイミングで、どのトランザクションの中で動いているか」を意識しないと、今回のように無関係に見える処理を道連れにします。同じような500エラーで悩んでいる方の参考になれば嬉しいです。