Skip to content

Conversation

@ronickg
Copy link
Contributor

@ronickg ronickg commented Oct 14, 2025

Instead of just getting errors like this: androidx.credentials.exceptions.domerrors.DataError@85a93b2 one now gets more info on what was the issue like so for instance : DomError: DataError - [50152] RP ID cannot be validated.

Summary by CodeRabbit

  • Refactor
    • Enhanced Android error messages for passkey registration and authentication failures by including the DOM error type and detailed message, improving clarity when issues occur.
  • Chores
    • Reduced console noise on web by removing debug logs when requested WebAuthn extensions are unavailable; behavior and alerts remain unchanged.
    • Streamlined the utility handling missing extensions to avoid logging internal keys while preserving user-facing warnings.

@coderabbitai
Copy link

coderabbitai bot commented Oct 14, 2025

Walkthrough

Updated Android error message formatting for DOM exceptions and removed console logging in two web code paths related to missing WebAuthn extensions. No control-flow changes or public API modifications.

Changes

Cohort / File(s) Summary of Changes
Android DOM exception formatting
android/src/main/java/.../ReactNativePasskeysModule.kt
Reformatted error strings for CreatePublicKeyCredentialDomException and GetPublicKeyCredentialDomException to include DOM error type and message: "DomError: - ". No control-flow changes.
Web logging cleanup
src/ReactNativePasskeysModule.web.ts, src/utils/warn-user-of-missing-webauthn-extensions.ts
Removed console logging in missing WebAuthn extension warnings; alert/behavior unchanged.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I twitch my whiskers, logs grow light,
Fewer squeaks in midnight bytes.
Android names its woes just right,
Web extensions out of sight.
I thump, content—cleaner night! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately conveys the two main updates—enhanced Android error logging and removal of unnecessary console logs—and is concise and directly related to the changes in the diff.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
android/src/main/java/expo/modules/passkeys/ReactNativePasskeysModule.kt (1)

100-104: Consider extracting duplicate DomException handling logic.

Both getRegistrationException and getAuthenticationException have identical logic for formatting DomException errors. Extracting this into a shared helper function would reduce duplication and improve maintainability.

For example, you could add:

private fun formatDomException(domError: Any, message: String?): String {
    val errorType = domError.javaClass.simpleName
    val errorMessage = message ?: "Unknown error"
    return "DomError: $errorType - $errorMessage"
}

Then use it in both functions:

 is CreatePublicKeyCredentialDomException -> {
-    val errorType = e.domError.javaClass.simpleName
-    val errorMessage = e.message ?: "Unknown error"
-    "DomError: $errorType - $errorMessage"
+    formatDomException(e.domError, e.message)
 }
 is GetPublicKeyCredentialDomException -> {
-    val errorType = e.domError.javaClass.simpleName
-    val errorMessage = e.message ?: "Unknown error"
-    "DomError: $errorType - $errorMessage"
+    formatDomException(e.domError, e.message)
 }

Also applies to: 131-135

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44c7c5c and 6e0611d.

📒 Files selected for processing (3)
  • android/src/main/java/expo/modules/passkeys/ReactNativePasskeysModule.kt (2 hunks)
  • src/ReactNativePasskeysModule.web.ts (0 hunks)
  • src/utils/warn-user-of-missing-webauthn-extensions.ts (0 hunks)
💤 Files with no reviewable changes (2)
  • src/ReactNativePasskeysModule.web.ts
  • src/utils/warn-user-of-missing-webauthn-extensions.ts
🔇 Additional comments (1)
android/src/main/java/expo/modules/passkeys/ReactNativePasskeysModule.kt (1)

100-104: Excellent improvement to error reporting!

The new format provides clear, human-readable error messages. The extraction of errorType and errorMessage is well-implemented, and the fallback to "Unknown error" is appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants