fix: graceful degradation when plugin not found - #63
Conversation
When a plugin that registered a PluginMailUser is not loaded (e.g., disabled or uninstalled), MailBox now gracefully degrades to DummyMailUser instead of throwing IllegalStateException. Changes: - UserConverter.readUser: catch IllegalStateException from PluginUser.toUser and return null - Base.getMails/getMail: handle null sender/receiver by creating fallback DummyMailUser - Base.getMailTemplate(s): handle null sender by creating fallback DummyMailUser This prevents crashes when viewing mails/templates that reference users from plugins that are not currently loaded.
Walkthroughユーザー変換時の Changesメールユーザーフォールバック
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces graceful error handling when converting users and loading mails from the database. Specifically, it catches IllegalStateException during user conversion to return null and provides fallback DummyMailUser instances when a sender or receiver is not found in the database. The review feedback highlights two critical issues: first, using the hardcoded key "unknown" for fallback users will trigger database unique constraint violations, so using the unique UUID string as the key is recommended; second, returning null from user conversion can cause downstream NullPointerExceptions in Base.getAllUsers() because the returned nulls are added to a list expected to contain only non-null elements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (sender == null) { | ||
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | ||
| } | ||
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | ||
| User receiver = getUser(receiverId); | ||
| if (receiver == null) { | ||
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation.
Why this is an issue:
- The
mailbox_userstable has aUNIQUEconstraint on theaddresscolumn (which stores the user's key):CREATE TABLE IF NOT EXISTS mailbox_users ( ... address TEXT DEFAULT NULL UNIQUE )
- When
mail.sender()ormail.receiver()is called (e.g., when rendering mails in the GUI), it triggersdatabase().getOrCreateUser(user). - If the plugin is missing,
getUser(uuid)returnsnull, sogetOrCreateUserattempts to update/insert the fallback user into the database. - If multiple fallback users are created with the same key
"unknown", the database will attempt to setaddress = "mailbox:unknown"for multiple rows, violating theUNIQUEconstraint and throwing aSQLException. This will crash the mail loading process entirely.
Solution:
Use the user's unique UUID string (e.g., senderId.toString() or receiverId.toString()) as the key value instead of "unknown". Since UUIDs are unique and valid key patterns (containing only lowercase letters, numbers, and hyphens), this completely avoids any database conflicts.
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | |
| } | |
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | |
| User receiver = getUser(receiverId); | |
| if (receiver == null) { | |
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", "unknown", null); | |
| } | |
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", senderId.toString(), null); | |
| } | |
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | |
| User receiver = getUser(receiverId); | |
| if (receiver == null) { | |
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", receiverId.toString(), null); | |
| } |
| if (sender == null) { | ||
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | ||
| } | ||
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | ||
| User receiver = getUser(receiverId); | ||
| if (receiver == null) { | ||
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation. Please use the unique UUID string as the key value instead.
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | |
| } | |
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | |
| User receiver = getUser(receiverId); | |
| if (receiver == null) { | |
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", "unknown", null); | |
| } | |
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", senderId.toString(), null); | |
| } | |
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | |
| User receiver = getUser(receiverId); | |
| if (receiver == null) { | |
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", receiverId.toString(), null); | |
| } |
| if (sender == null) { | ||
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | ||
| } | ||
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | ||
| User receiver = getUser(receiverId); | ||
| if (receiver == null) { | ||
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation. Please use the unique UUID string as the key value instead.
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | |
| } | |
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | |
| User receiver = getUser(receiverId); | |
| if (receiver == null) { | |
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", "unknown", null); | |
| } | |
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", senderId.toString(), null); | |
| } | |
| UUID receiverId = UUID.fromString(rs.getString("receiver")); | |
| User receiver = getUser(receiverId); | |
| if (receiver == null) { | |
| receiver = DummyMailUser.createUser(receiverId, "Unknown Receiver", receiverId.toString(), null); | |
| } |
| if (sender == null) { | ||
| sender = DummyMailUser.createUser(senderId != null ? senderId : DummyMailUser.SYSTEM_UUID, "Unknown Sender", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation. Please use the unique UUID string as the key value instead.
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId != null ? senderId : DummyMailUser.SYSTEM_UUID, "Unknown Sender", "unknown", null); | |
| } | |
| if (sender == null) { | |
| UUID fallbackId = senderId != null ? senderId : DummyMailUser.SYSTEM_UUID; | |
| sender = DummyMailUser.createUser(fallbackId, "Unknown Sender", fallbackId.toString(), null); | |
| } |
| if (sender == null) { | ||
| sender = DummyMailUser.createUser(senderId != null ? senderId : DummyMailUser.SYSTEM_UUID, "Unknown Sender", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation. Please use the unique UUID string as the key value instead.
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId != null ? senderId : DummyMailUser.SYSTEM_UUID, "Unknown Sender", "unknown", null); | |
| } | |
| if (sender == null) { | |
| UUID fallbackId = senderId != null ? senderId : DummyMailUser.SYSTEM_UUID; | |
| sender = DummyMailUser.createUser(fallbackId, "Unknown Sender", fallbackId.toString(), null); | |
| } |
| if (sender1 == null) { | ||
| sender1 = DummyMailUser.createUser(senderId != null ? senderId : DummyMailUser.SYSTEM_UUID, "Unknown Sender", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation. Please use the unique UUID string as the key value instead.
| if (sender1 == null) { | |
| sender1 = DummyMailUser.createUser(senderId != null ? senderId : DummyMailUser.SYSTEM_UUID, "Unknown Sender", "unknown", null); | |
| } | |
| if (sender1 == null) { | |
| UUID fallbackId = senderId != null ? senderId : DummyMailUser.SYSTEM_UUID; | |
| sender1 = DummyMailUser.createUser(fallbackId, "Unknown Sender", fallbackId.toString(), null); | |
| } |
| if (sender == null) { | ||
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | ||
| } |
There was a problem hiding this comment.
Using a hardcoded key value of "unknown" for all fallback DummyMailUsers will cause a database constraint violation. Please use the unique UUID string as the key value instead.
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", "unknown", null); | |
| } | |
| if (sender == null) { | |
| sender = DummyMailUser.createUser(senderId, "Unknown Sender", senderId.toString(), null); | |
| } |
| try { | ||
| return baseUser.toUser(uuid, key); | ||
| } catch (IllegalStateException e) { | ||
| // Plugin not found or other state issues - return null gracefully | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Returning null from readUser when a plugin is not found is a great way to handle graceful degradation. However, please note that Base.getAllUsers() (line 304 in Base.java) directly adds the result of readUser(rs) to a list of type List<@NotNull User> without checking for null:
while (rs.next()) {
users.add(readUser(rs));
}This will result in null elements being added to a list that is contractually specified to contain only non-null Users, which can lead to unexpected NullPointerExceptions downstream when iterating over the list.
Recommendation:
Please update Base.getAllUsers() to filter out null users before adding them to the list:
User user = readUser(rs);
if (user != null) {
users.add(user);
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java (1)
326-338: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win重複するダミーユーザー生成処理の共通化
ユーザー取得時の null チェックと
DummyMailUserへのフォールバック処理が複数の箇所で重複しています。一部の処理（例:getMailTemplateのsenderId != null ? senderId : DummyMailUser.SYSTEM_UUID）の冗長な記述も含め、一貫したヘルパーメソッドへ抽出することでコードの見通しを良くすることができます。以下のようなヘルパーメソッドを追加し、それぞれの箇所を置き換えることをご検討ください。
💡 ヘルパーメソッドの追加例
private `@NotNull` User getUserOrDefault(`@Nullable` UUID userId, `@NotNull` String defaultName) { UUID id = userId != null ? userId : DummyMailUser.SYSTEM_UUID; User user = getUser(id); if (user == null) { return DummyMailUser.createUser(id, defaultName, "unknown", null); } return user; }呼び出し側の例:
UUID senderId = Optional.ofNullable(rs.getString("sender")).map(UUID::fromString).orElse(null); User sender = getUserOrDefault(senderId, "Unknown Sender"); UUID receiverId = UUID.fromString(rs.getString("receiver")); User receiver = getUserOrDefault(receiverId, "Unknown Receiver");
paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L326-L338: 追加したヘルパーメソッドを使用するようリファクタリングします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L353-L365: 追加したヘルパーメソッドを使用するようリファクタリングします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L411-L423: 追加したヘルパーメソッドを使用するようリファクタリングします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L511-L518: 追加したヘルパーメソッドを使用するようリファクタリングします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L544-L551: 追加したヘルパーメソッドを使用するようリファクタリングします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L577-L584: 追加したヘルパーメソッドを使用するようリファクタリングします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java#L610-L617: 追加したヘルパーメソッドを使用するようリファクタリングします。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java` around lines 326 - 338, Base.java の各所で重複しているユーザー取得と DummyMailUser フォールバック処理を共通化してください。Base に getUserOrDefault 相当のヘルパーメソッドを追加し、null の UUID には SYSTEM_UUID を使い、取得できない場合は指定名のダミーユーザーを返すようにします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java の326-338、353-365、411-423、511-518、544-551、577-584、610-617では、それぞれ既存の個別処理をこのヘルパー呼び出しへ置き換え、getMailTemplate を含む冗長な senderId 処理も統一してください。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java`:
- Around line 326-338: Base.java の各所で重複しているユーザー取得と DummyMailUser
フォールバック処理を共通化してください。Base に getUserOrDefault 相当のヘルパーメソッドを追加し、null の UUID には
SYSTEM_UUID
を使い、取得できない場合は指定名のダミーユーザーを返すようにします。paper/src/main/java/net/sabafly/mailBox/database/impl/Base.java
の326-338、353-365、411-423、511-518、544-551、577-584、610-617では、それぞれ既存の個別処理をこのヘルパー呼び出しへ置き換え、getMailTemplate
を含む冗長な senderId 処理も統一してください。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc77934b-647e-4440-a785-92b5fa35e239
📒 Files selected for processing (2)
paper/src/main/java/net/sabafly/mailBox/database/UserConverter.javapaper/src/main/java/net/sabafly/mailBox/database/impl/Base.java
1 participant
Problem
When a plugin that registered a
PluginMailUseris not loaded (e.g., disabled or uninstalled), MailBox throwsIllegalStateException: Plugin not found for namespace: xxxand crashes when trying to view mails/templates.Solution
Gracefully degrade to
DummyMailUserinstead of crashing:IllegalStateExceptionfromPluginUser.toUser()and returnnullDummyMailUserDummyMailUserUse Case
This commonly happens when:
Testing
Tested with TransferStation plugin disabled - MailBox now shows "Unknown Sender/Receiver" instead of crashing.
Summary by CodeRabbit