Skip to content

fix(native-auth-ops): remove exceptions from logs in KDF restart function#45

Merged
CharlVS merged 3 commits intodevfrom
bugfix/native-kdf-login
Apr 7, 2025
Merged

fix(native-auth-ops): remove exceptions from logs in KDF restart function#45
CharlVS merged 3 commits intodevfrom
bugfix/native-kdf-login

Conversation

@takenagain
Copy link
Copy Markdown
Contributor

@takenagain takenagain commented Apr 6, 2025

Removes findExceptionsInLog from the _restartKdf function, which is re-throwing connection errors from RPCs like the one below. These errors are innocuous errors, usually seen while KDF is restarting during a login.

Also adds logic to wait for the RPC to be up in native kdfMain rather than a static 500ms delay.

I/flutter (22277): Error getting KDF version: ClientException with SocketException: Connection refused (OS Error: Connection refused, errno = 111), address = localhost, port = 42740, uri=http://localhost:7783

The `findExceptionsInLog` function called in the _restartKdf appears to be finding innocuous errors and re-throwing them. 

The one being caught on my machine is a socket connection error from an RPC, version, likely while KDF is starting or stopping.

Rethrowing exceptions from logs seems out-of-place in the restart kdf function.
@takenagain takenagain added the bug Something isn't working label Apr 6, 2025
@takenagain takenagain self-assigned this Apr 6, 2025
Copilot AI review requested due to automatic review settings April 6, 2025 20:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request. See here for more details.

Files not reviewed (7)
  • packages/komodo_defi_framework/app_build/build_config.json: Language not supported
  • packages/komodo_defi_framework/lib/src/operations/kdf_operations_native.dart: Language not supported
  • packages/komodo_defi_local_auth/lib/src/auth/auth_service_kdf_extension.dart: Language not supported
  • packages/komodo_defi_sdk/example/ios/Runner.xcodeproj/project.pbxproj: Language not supported
  • packages/komodo_defi_sdk/example/ios/Runner/Info.plist: Language not supported
  • packages/komodo_defi_sdk/example/web/kdf/res/kdf_wrapper.dart: Language not supported
  • packages/komodo_defi_types/lib/src/utils/security_utils.dart: Language not supported

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 6, 2025

Walkthrough

This pull request updates the system’s configuration and control flow in multiple areas. The build configuration now references a new commit hash for the bundled coins repository. The KDF operations have been modified to replace a fixed delay with a loop that checks for RPC server readiness and simplify error handling in the restart routine. In addition, placeholder properties were added to iOS build phases, UI keys were re-ordered in the Info.plist, a header comment was updated in a web resource, and the sensitive key list in security utilities was expanded.

Changes

File(s) Change Summary
packages/komodo_defi_framework/app_build/build_config.json Updated bundled_coins_repo_commit value with a new commit hash.
packages/komodo_defi_framework/lib/src/operations/kdf_operations_native.dart
packages/komodo_defi_local_auth/lib/src/auth/auth_service_kdf_extension.dart
Modified KDF logic: replaced a fixed delay with a loop for RPC readiness in kdfMain and simplified _restartKdf by removing log stream error handling.
packages/komodo_defi_sdk/example/ios/Runner.xcodeproj/project.pbxproj
packages/komodo_defi_sdk/example/ios/Runner/Info.plist
Added empty inputPaths and outputPaths arrays in build phases; removed and re-added UI keys without altering their values.
packages/komodo_defi_sdk/example/web/kdf/res/kdf_wrapper.dart Added an extra comment regarding a future migration of the KDF JavaScript bootstrapper to Dart.
packages/komodo_defi_types/lib/src/utils/security_utils.dart Added a comment and a new key (wallet_password) to the sensitive keys list in the censored method.

Sequence Diagram(s)

sequenceDiagram
    participant KDF as KDF Operation
    participant RPC as RPC Server
    KDF->>RPC: Check RPC status
    alt Not ready
        loop Up to 20 attempts
            KDF->>RPC: Check RPC status (retry)
            RPC-->>KDF: Status: not ready
        end
        KDF->>KDF: Log warning (if still not ready)
    else Ready
        RPC-->>KDF: Status: rpcIsUp
    end
    KDF->>KDF: Proceed with KDF operation
Loading

Poem

I'm a bunny, hopping through code all day,
Skipping fixed delays that once led astray,
Checking each RPC with a loop so keen,
Simplifying KDF like a springtime scene,
With every change, I dance in delight!
🐰✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fdcc0e and f112017.

📒 Files selected for processing (7)
  • packages/komodo_defi_framework/app_build/build_config.json (1 hunks)
  • packages/komodo_defi_framework/lib/src/operations/kdf_operations_native.dart (1 hunks)
  • packages/komodo_defi_local_auth/lib/src/auth/auth_service_kdf_extension.dart (1 hunks)
  • packages/komodo_defi_sdk/example/ios/Runner.xcodeproj/project.pbxproj (2 hunks)
  • packages/komodo_defi_sdk/example/ios/Runner/Info.plist (1 hunks)
  • packages/komodo_defi_sdk/example/web/kdf/res/kdf_wrapper.dart (1 hunks)
  • packages/komodo_defi_types/lib/src/utils/security_utils.dart (2 hunks)
🔇 Additional comments (9)
packages/komodo_defi_sdk/example/web/kdf/res/kdf_wrapper.dart (1)

2-3: Added helpful context for future development

The comment addition provides valuable context about a potential future direction for this file - migrating the KDF JavaScript bootstrapper to Dart and compiling it to JavaScript.

packages/komodo_defi_framework/app_build/build_config.json (1)

66-66:

Details

✅ Verification successful

Updated bundled_coins_repo_commit hash

The commit hash update for the bundled coins repository is appropriate. This will ensure the application references the latest version of the repository during the build process.


🏁 Script executed:

#!/bin/bash
# Check if the commit hash exists in the KomodoPlatform/coins repository
gh api repos/KomodoPlatform/coins/commits/57c196726f67cabb242bfce9b39ab04b10001ec5 --jq '.sha'

Length of output: 135


Commit Hash Verified and Approved

  • The commit hash 57c196726f67cabb242bfce9b39ab04b10001ec5 in packages/komodo_defi_framework/app_build/build_config.json (line 66) was verified using the GitHub CLI and correctly points to an existing commit in the KomodoPlatform/coins repository.
  • This update ensures that the application will reference the latest version during the build process.
packages/komodo_defi_types/lib/src/utils/security_utils.dart (2)

74-74: Good enhancement suggestion for pattern matching

The TODO comment suggests a valuable enhancement to add regex or wildcard support for matching keys containing "password" or "key", which would improve the censoring mechanism's coverage.


90-90: Enhanced security by adding wallet_password to sensitive keys

Adding 'wallet_password' to the list of sensitive keys that get censored strengthens the application's security posture by ensuring this sensitive information isn't leaked in logs or outputs.

packages/komodo_defi_framework/lib/src/operations/kdf_operations_native.dart (1)

177-202: Improved RPC server readiness detection

This change replaces a fixed delay with an active polling mechanism to ensure the RPC server is fully operational before continuing. This is a significant improvement that addresses potential race conditions where KDF operations might start before the RPC server is ready to accept requests.

Key improvements:

  • Polls RPC status up to 20 times instead of using a fixed delay
  • Provides debug logging about readiness timing
  • Warns if RPC server doesn't become ready, improving diagnostics
packages/komodo_defi_sdk/example/ios/Runner.xcodeproj/project.pbxproj (2)

297-298: Added standardized placeholders for resource paths.

These empty arrays for inputPaths and outputPaths in the [CP] Copy Pods Resources build phase provide standardized placeholders that may be used for future path entries while maintaining consistency in the Xcode project structure.

Also applies to: 303-304


377-378: Added standardized placeholders for framework paths.

Similar to the resources build phase, these empty arrays for inputPaths and outputPaths in the [CP] Embed Pods Frameworks build phase maintain consistency in the project structure and follow Xcode conventions for build configuration.

Also applies to: 383-384

packages/komodo_defi_sdk/example/ios/Runner/Info.plist (1)

36-41: Restored UI configuration keys in consistent order.

The UI-related keys (UIApplicationSupportsIndirectInputEvents, UILaunchStoryboardName, and UIMainStoryboardFile) have been reordered or re-added with their original values. This change maintains the same functionality while potentially improving the structure of the plist file.

packages/komodo_defi_local_auth/lib/src/auth/auth_service_kdf_extension.dart (1)

78-90: Streamlined KDF restart logic by removing exception logging.

The _restartKdf method has been simplified by removing the complex exception logging and collection mechanism. The new implementation:

  1. Stops the KDF service
  2. Attempts to start KDF with the provided configuration
  3. Throws an exception immediately if starting fails
  4. Waits for the KDF RPC to be up and running

This change aligns with the PR objective of removing exceptions from logs in the KDF restart function, resulting in cleaner, more maintainable code with the same functional outcome.


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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@takenagain
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@takenagain takenagain requested a review from CharlVS April 6, 2025 21:09
@CharlVS CharlVS merged commit 67e531e into dev Apr 7, 2025
1 of 2 checks passed
@CharlVS CharlVS deleted the bugfix/native-kdf-login branch April 7, 2025 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants