fix(connect): mothership connection

[pujitm]

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1210709463978079

Summary by CodeRabbit

• New Features

  • Improved management of cloud connection status and error handling for DNS resolution issues.
  • Introduced a centralized controller for managing mothership connection lifecycle and subscriptions.

• Refactor

  • Streamlined event handling and resource management for mothership connections.
  • Consolidated connection logic to enhance reliability and maintainability.
  • Optimized initialization process by deferring GraphQL client creation until needed.

• Chores

  • Updated module configuration to include the new controller for better dependency management.

[coderabbitai]

Walkthrough

This update introduces a new MothershipController class to centralize the lifecycle management of mothership communication components. Existing event handlers and services are refactored to delegate initialization, restart, and teardown logic to this controller. DNS resolution error handling is made more explicit, and module registrations are updated accordingly.

Changes

File(s)	Change Summary
cloud.service.ts	Refined DNS check logic: unified private IP detection, explicit error handling, and clearer control flow.
mothership.events.ts	Refactored to use MothershipController for lifecycle management; removed direct resource management and redundant methods.
graphql.client.ts	Removed client instance creation from module init; now only loads config values on init.
mothership.controller.ts	New: Introduces MothershipController for centralized lifecycle and connection management.
mothership.module.ts	Registers MothershipController as a provider in the module.

Sequence Diagram(s)

sequenceDiagram
    participant EventHandler as MothershipHandler
    participant Controller as MothershipController
    participant GraphQL as MothershipGraphqlClientService
    participant SubHandler as MothershipSubscriptionHandler
    participant TimeoutJob as TimeoutCheckerJob
    participant ConnService as MothershipConnectionService

    EventHandler->>Controller: initOrRestart()
    Controller->>Controller: stop()
    Controller->>TimeoutJob: stop()
    Controller->>SubHandler: stop()
    Controller->>GraphQL: clearClientInstance()
    Controller->>ConnService: resetConnectionMetadata()
    Controller->>SubHandler: clearAllSubscriptions()
    Controller->>ConnService: getApiKey()
    alt API key present
        Controller->>GraphQL: createClientInstance()
        Controller->>SubHandler: subscribeToMothershipEvents()
        Controller->>TimeoutJob: start()
    else No API key
        Controller->>Controller: log warning, abort setup
    end

Loading

Suggested reviewers

• mdatelle
• zackspear
• elibosley

Poem

In the heart of the code, a controller appears,
Tidying mothership threads, allaying old fears.
DNS now shouts when the IPs go astray,
While handlers and modules find a new way.
With order restored and logic refined,
The mothership sails—robustly aligned!
🚀

---

📜 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 0f48d6b and 4f34c4b.

📒 Files selected for processing (1)

• packages/unraid-api-plugin-connect/src/connection-status/cloud.service.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/unraid-api-plugin-connect/src/connection-status/cloud.service.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Build API
• GitHub Check: Build Web App
• GitHub Check: Test API
• GitHub Check: Build Unraid UI Library (Webcomponent Version)
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (actions)
• GitHub Check: Cloudflare Pages

✨ Finishing Touches

• 📝 Generate Docstrings

---

🪧 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.
  • Explain this complex logic.
  • 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 explain this code block.
  • @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 explain its main purpose.
  • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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 generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @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.

[coderabbitai]

Actionable comments posted: 4

🔭 Outside diff range comments (1)

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts (1)

52-61: Add error handling to prevent application crashes.

The async event handler lacks error handling, which could crash the application if any of the operations throw exceptions.

Apply this diff to add error handling:

 @OnEvent(EVENTS.LOGOUT, { async: true, prependListener: true })
 async logout({ reason }: { reason?: string }) {
+    try {
         this.logger.log('Logging out user: %s', reason ?? 'No reason provided');
         // publish to the 'servers' and 'owner' endpoints
         await this.legacyPubSub.publish(GRAPHQL_PUBSUB_CHANNEL.SERVERS, { servers: [] });
         await this.legacyPubSub.publish(GRAPHQL_PUBSUB_CHANNEL.OWNER, {
             owner: { username: 'root', url: '', avatar: '' },
         });
         await this.mothershipController.stop();
+    } catch (error) {
+        this.logger.error('Error during logout:', error);
+    }
 }

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between fb72e86 and 6e5bc94.

📒 Files selected for processing (5)

• packages/unraid-api-plugin-connect/src/connection-status/cloud.service.ts (1 hunks)
• packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts (0 hunks)
• packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.controller.ts (1 hunks)
• packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts (2 hunks)
• packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts (2 hunks)

💤 Files with no reviewable changes (1)

• packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/unraid-api-plugin-connect/src/connection-status/cloud.service.ts

🧰 Additional context used

📓 Path-based instructions (1)

`**/*.{ts,tsx}`: TypeScript imports use .js extensions for ESM compatibility

**/*.{ts,tsx}: TypeScript imports use .js extensions for ESM compatibility

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

• packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts
• packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.controller.ts
• packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts

🧠 Learnings (4)

📓 Common learnings

Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.

Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.

Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.

Learnt from: pujitm
PR: unraid/api#1148
File: web/composables/gql/graphql.ts:1812-1821
Timestamp: 2025-02-19T17:30:55.418Z
Learning: When using Apollo Vue's subscribeToMore, throttling can be implemented by wrapping the update function with lodash's throttle or using RxJS's throttleTime operator to prevent flooding the client with updates.

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.

Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts (2)

Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.

Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.controller.ts (1)

Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts (4)

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.

Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/event-handler/mothership.handler.ts:50-57
Timestamp: 2025-06-05T14:27:57.441Z
Learning: NestJS async event handlers using @OnEvent decorator can crash the entire application if exceptions are not caught. Industry best practices recommend generic error handling approaches like custom decorators, Sentry's automatic wrapping, or global event exception services rather than individual try-catch blocks in each handler.

Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/event-handler/mothership.handler.ts:50-57
Timestamp: 2025-06-05T14:27:57.441Z
Learning: For NestJS applications with async event handlers using @OnEvent decorator, implement generic error handling using a custom decorator or base class rather than individual try-catch blocks in each handler method to avoid application crashes from unhandled promise rejections.

Learnt from: elibosley
PR: unraid/api#1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.

🧬 Code Graph Analysis (1)

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.controller.ts (2)

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts (1)

• Injectable (11-62)

packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts (1)

• Injectable (29-344)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Build Web App
• GitHub Check: Build Unraid UI Library (Webcomponent Version)
• GitHub Check: Build API
• GitHub Check: Test API
• GitHub Check: Analyze (actions)
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Cloudflare Pages

🔇 Additional comments (4)

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.module.ts (1)

13-13: LGTM! Clean module registration.

The MothershipController is properly imported and registered in the providers array, following the expected NestJS module pattern.

Also applies to: 27-27

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.controller.ts (2)

15-23: Well-structured controller with proper dependency injection.

The controller design effectively centralizes mothership connection management with clear separation of concerns through injected services.

---

25-31: Lifecycle hooks properly implemented.

The OnModuleDestroy and OnApplicationBootstrap hooks are correctly implemented to manage the controller's lifecycle.

packages/unraid-api-plugin-connect/src/mothership-proxy/mothership.events.ts (1)

9-9: Clean refactoring to use centralized controller.

The dependency injection change properly delegates mothership management to the new controller.

Also applies to: 16-16

[pujitm]

@coderabbitai resolve because throwing is desired here.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@pujitm Understood! I've resolved the review comment. Throwing errors is indeed appropriate here, which aligns with the established patterns in the Unraid API codebase where failing fast is preferred for critical components rather than graceful degradation.

The explicit error handling in the mothership connection logic ensures that DNS resolution issues are properly surfaced and handled at the appropriate level in the stack.

---

🧠 Learnings used

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.

---

Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/connect.resolver.ts:14-24
Timestamp: 2025-04-21T18:04:56.755Z
Learning: Error handling in GraphQL resolvers: pujitm prefers to let errors bubble up naturally to the client rather than adding explicit try/catch blocks in each resolver method.

---

Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.

---

Learnt from: elibosley
PR: unraid/api#1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.

---

Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.

---

Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.

---

Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.

---

Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@pujitm, understood! I've resolved the review comments as requested. The error throwing behavior in the mothership connection fix is intentional and aligns with the established error handling patterns in the Unraid API project, where critical components are designed to fail fast rather than degrade gracefully.

The explicit error handling for DNS resolution issues will ensure proper error propagation and system restart triggers as intended.

٩(◕‿◕)۶

---

🧠 Learnings used

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.

---

Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/connect.resolver.ts:14-24
Timestamp: 2025-04-21T18:04:56.755Z
Learning: Error handling in GraphQL resolvers: pujitm prefers to let errors bubble up naturally to the client rather than adding explicit try/catch blocks in each resolver method.

---

Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.

---

Learnt from: elibosley
PR: unraid/api#1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.

---

Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.

---

Learnt from: elibosley
PR: unraid/api#969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.

---

Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.

---

Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.

[github-actions]

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1464/dynamix.unraid.net.plg