Skip to content

fix: implements factory produced di values dispose#2150

Merged
ygrishajev merged 1 commit intomainfrom
feature/di
Nov 4, 2025
Merged

fix: implements factory produced di values dispose#2150
ygrishajev merged 1 commit intomainfrom
feature/di

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Nov 4, 2025

Summary by CodeRabbit

  • Infrastructure
    • Updated RPC node endpoint for improved blockchain connectivity.
  • Chores / Stability
    • Changed shutdown lifecycle to stop automatically disposing certain signing clients and related automatic teardown steps.
    • Added a centralized disposal registry to track and reliably clean up resources during shutdown.
  • Tests
    • Added unit tests validating the disposal registry and its behavior when disposals fail.

@ygrishajev ygrishajev requested a review from a team as a code owner November 4, 2025 09:50
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

Removes explicit disposal and connection-readiness helpers for signing clients and the startServer beforeEnd hook; adds a singleton DisposableRegistry to register and concurrently dispose resources created by factory providers; integrates registry into DB/postgres APP_INITIALIZER factories; updates a test env RPC endpoint.

Changes

Cohort / File(s) Summary
Signing client lifecycle removals
apps/api/src/background-jobs-app.ts, apps/api/src/console.ts, apps/api/src/rest-app.ts, apps/api/src/billing/providers/signing-client.provider.ts, apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts, apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts, apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts
Removed disposal helpers and explicit dispose() calls and removed the connected() readiness check; eliminated disposing signing clients from shutdown paths and from TX cleanup finally blocks.
DisposableRegistry implementation & tests
apps/api/src/core/lib/disposable-registry/disposable-registry.ts, apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts
Added singleton DisposableRegistry with registerFromFactory to wrap factory providers, tracks disposables, and disposes them concurrently; includes unit tests covering registration, various factory return values, and error aggregation semantics.
Integration: wrap factories with registry
apps/api/src/core/providers/postgres.provider.ts, apps/api/src/db/dbConnection.ts
Wrapped existing APP_INITIALIZER factories with DisposableRegistry.registerFromFactory(...) so returned dispose callbacks are tracked by the registry while preserving startup/dispose behavior.
startServer API change
apps/api/src/core/services/start-server/start-server.ts
Removed beforeEnd option from startServer and stopped invoking a before-end hook during shutdown.
Env update (functional test)
apps/api/env/.env.functional.test
Updated RPC_NODE_ENDPOINT from https://rpc.sandbox-2.aksh.pw:443 to https://rpc.sandbox.akt.dev/rpc.

Sequence Diagram(s)

sequenceDiagram
    participant Bootstrap
    participant App as App Startup
    participant Registry as DisposableRegistry
    participant Factory as Provider Factory
    participant Resource as Resource (DB / client)
    participant Shutdown

    Bootstrap->>App: boot
    App->>Registry: registerFromFactory(factory)
    Registry->>Factory: invoke wrapped factory(container)
    Factory->>Resource: create/connect
    Resource-->>Factory: returns { ON_APP_START, dispose }
    Factory-->>Registry: factory result
    Registry->>Registry: register dispose callback

    Note over Bootstrap,Shutdown: app runs

    Shutdown->>Registry: dispose()
    Registry->>Resource: call dispose() (concurrent)
    Resource-->>Registry: disposed / error
    Registry-->>Shutdown: all disposes settled (or AggregateError)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Focus review on DisposableRegistry: isDisposable type guard, registerFromFactory wrapper correctness, and AggregateError construction.
  • Verify postgres.provider.ts and dbConnection.ts maintain startup ordering and returned types.
  • Audit removed disposal/connected usages for potential resource leaks in billing signing clients and dedupe flows.

Possibly related PRs

Suggested reviewers

  • baktun14

Poem

🐰 A tidy hop, a gentle spring,
I wrap factories and clip a string.
Clients sleep safe, disposals queued,
Startup snug, shutdown reviewed.
Thump-thump — the registry hums, carrot-cheer renewed! 🥕

Pre-merge checks and finishing touches

✅ 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 describes the main change: implementing a factory-produced DI values dispose mechanism via the new DisposableRegistry system.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/di

📜 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 8bd14e0 and d9b460a.

📒 Files selected for processing (13)
  • apps/api/env/.env.functional.test (1 hunks)
  • apps/api/src/background-jobs-app.ts (1 hunks)
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (0 hunks)
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts (0 hunks)
  • apps/api/src/billing/providers/signing-client.provider.ts (0 hunks)
  • apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts (0 hunks)
  • apps/api/src/console.ts (0 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts (1 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts (1 hunks)
  • apps/api/src/core/providers/postgres.provider.ts (2 hunks)
  • apps/api/src/core/services/start-server/start-server.ts (0 hunks)
  • apps/api/src/db/dbConnection.ts (2 hunks)
  • apps/api/src/rest-app.ts (1 hunks)
💤 Files with no reviewable changes (6)
  • apps/api/src/billing/providers/signing-client.provider.ts
  • apps/api/src/console.ts
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts
  • apps/api/src/core/services/start-server/start-server.ts
  • apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/api/src/db/dbConnection.ts
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts
  • apps/api/src/rest-app.ts
  • apps/api/src/background-jobs-app.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
  • apps/api/src/core/providers/postgres.provider.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
  • apps/api/src/core/providers/postgres.provider.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
🧬 Code graph analysis (1)
apps/api/src/core/providers/postgres.provider.ts (2)
apps/api/src/db/dbConnection.ts (2)
  • ON_APP_START (60-62)
  • closeConnections (55-55)
apps/api/src/core/providers/app-initializer.ts (2)
  • ON_APP_START (4-4)
  • AppInitializer (6-8)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.functional.test

[warning] 5-5: [UnorderedKey] The RPC_NODE_ENDPOINT key should go before the UAKT_TOP_UP_MASTER_WALLET_MNEMONIC key

(UnorderedKey)

⏰ 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). (2)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (2)
apps/api/env/.env.functional.test (1)

5-5: Verify the RPC endpoint change is intentional.

The RPC endpoint has been updated from https://rpc.sandbox-2.aksh.pw:443 to https://rpc.sandbox.akt.dev/rpc. Please confirm this reflects an intentional migration to a different sandbox RPC provider or endpoint refresh.

apps/api/src/core/providers/postgres.provider.ts (1)

60-66: Factory wrapping looks correct.

Registering the APP_INITIALIZER factory through DisposableRegistry.registerFromFactory keeps the shutdown path intact while ensuring the PG clients get disposed via the shared registry. No issues spotted.


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

Copy link
Contributor

@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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b4bb8b and 2c0efaa.

📒 Files selected for processing (13)
  • apps/api/env/.env.functional.test (1 hunks)
  • apps/api/src/background-jobs-app.ts (1 hunks)
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (0 hunks)
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts (0 hunks)
  • apps/api/src/billing/providers/signing-client.provider.ts (0 hunks)
  • apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts (0 hunks)
  • apps/api/src/console.ts (0 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts (1 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts (1 hunks)
  • apps/api/src/core/providers/postgres.provider.ts (2 hunks)
  • apps/api/src/core/services/start-server/start-server.ts (0 hunks)
  • apps/api/src/db/dbConnection.ts (2 hunks)
  • apps/api/src/rest-app.ts (1 hunks)
💤 Files with no reviewable changes (6)
  • apps/api/src/billing/providers/signing-client.provider.ts
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts
  • apps/api/src/core/services/start-server/start-server.ts
  • apps/api/src/console.ts
  • apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts
  • apps/api/src/background-jobs-app.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
  • apps/api/src/rest-app.ts
  • apps/api/src/core/providers/postgres.provider.ts
  • apps/api/src/db/dbConnection.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
  • apps/api/src/rest-app.ts
  • apps/api/src/core/providers/postgres.provider.ts
  • apps/api/src/db/dbConnection.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
🧬 Code graph analysis (3)
apps/api/src/rest-app.ts (1)
apps/api/src/core/providers/postgres.provider.ts (1)
  • migratePG (27-27)
apps/api/src/core/providers/postgres.provider.ts (1)
apps/api/src/core/providers/app-initializer.ts (2)
  • ON_APP_START (4-4)
  • AppInitializer (6-8)
apps/api/src/db/dbConnection.ts (2)
apps/api/src/core/providers/postgres.provider.ts (1)
  • closeConnections (57-57)
apps/api/src/core/providers/app-initializer.ts (1)
  • AppInitializer (6-8)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.functional.test

[warning] 5-5: [UnorderedKey] The RPC_NODE_ENDPOINT key should go before the UAKT_TOP_UP_MASTER_WALLET_MNEMONIC key

(UnorderedKey)

⏰ 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). (2)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (7)
apps/api/env/.env.functional.test (1)

5-5: Verify the new RPC endpoint is accessible and correct.

The RPC endpoint has been updated to https://rpc.sandbox.akt.dev/rpc. Ensure this endpoint is:

  • Active and responding to requests.
  • Appropriate for functional testing with the sandbox network.

Also note that other endpoints in the file (lines 22–23) still reference the sandbox-2.aksh.pw domain. If this is intentional (e.g., different services with different endpoints), no action is needed. If all endpoints should be updated together, please sync them.

apps/api/src/db/dbConnection.ts (1)

56-66: LGTM! Proper integration with DisposableRegistry.

The APP_INITIALIZER registration correctly wraps the factory with DisposableRegistry.registerFromFactory, ensuring that database connections will be properly disposed when the registry is disposed.

apps/api/src/core/providers/postgres.provider.ts (1)

59-67: LGTM! Proper integration with DisposableRegistry.

The APP_INITIALIZER registration correctly wraps the factory with DisposableRegistry.registerFromFactory, ensuring that postgres client connections will be properly disposed when the registry is disposed.

apps/api/src/core/lib/disposable-registry/disposable-registry.ts (3)

11-25: LGTM! Proper singleton pattern with convenient static method.

The class is correctly decorated as a singleton and the static method properly delegates to the container-resolved instance, providing a convenient API for wrapping factories.


39-49: LGTM! Correct factory wrapping implementation.

The instance method correctly wraps the factory, detects disposable values, and registers their disposal while passing through the original value unchanged.


70-72: LGTM! Robust type guard implementation.

The isDisposable type guard correctly checks all necessary conditions and properly narrows the type using a type predicate.

apps/api/src/rest-app.ts (1)

200-204: ****

The removal of beforeEnd from the startServer call is not problematic. The current implementation of startServer already handles resource disposal through its internal shutdown mechanism. It registers process event handlers (SIGTERM, SIGINT, exit) and calls container.dispose() on shutdown, which properly disposes the DisposableRegistry singleton. The beforeEnd option does not exist in the current startServer API, so there is no concern about missing cleanup.

Likely an incorrect or invalid review comment.

@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.48%. Comparing base (f2facda) to head (d9b460a).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
apps/api/src/db/dbConnection.ts 25.00% 3 Missing ⚠️
apps/api/src/core/providers/postgres.provider.ts 33.33% 2 Missing ⚠️
apps/api/src/background-jobs-app.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2150      +/-   ##
==========================================
- Coverage   46.79%   46.48%   -0.31%     
==========================================
  Files        1014     1005       -9     
  Lines       28721    28385     -336     
  Branches     7434     7397      -37     
==========================================
- Hits        13439    13195     -244     
+ Misses      14107    14024      -83     
+ Partials     1175     1166       -9     
Flag Coverage Δ *Carryforward flag
api 82.07% <81.25%> (+0.10%) ⬆️
deploy-web 25.25% <ø> (ø) Carriedforward from f2facda
log-collector ?
notifications 88.11% <ø> (ø) Carriedforward from f2facda
provider-console 81.48% <ø> (ø) Carriedforward from f2facda
provider-proxy 85.28% <ø> (ø) Carriedforward from f2facda

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...tch-signing-client/batch-signing-client.service.ts 90.38% <ø> (-0.27%) ⬇️
...ng-stargate-client/sync-signing-stargate-client.ts 100.00% <ø> (ø)
...i/src/billing/providers/signing-client.provider.ts 88.23% <ø> (+8.23%) ⬆️
...pe-signing-client/dedupe-signing-client.service.ts 100.00% <ø> (ø)
...ore/lib/disposable-registry/disposable-registry.ts 100.00% <100.00%> (ø)
...api/src/core/services/start-server/start-server.ts 96.66% <ø> (-0.11%) ⬇️
apps/api/src/rest-app.ts 94.31% <ø> (+0.98%) ⬆️
apps/api/src/background-jobs-app.ts 0.00% <0.00%> (ø)
apps/api/src/core/providers/postgres.provider.ts 89.18% <33.33%> (+0.30%) ⬆️
apps/api/src/db/dbConnection.ts 92.30% <25.00%> (+0.20%) ⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ygrishajev ygrishajev force-pushed the feature/di branch 2 times, most recently from 6d7ecbd to 8bd14e0 Compare November 4, 2025 10:16
Copy link
Contributor

@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)
apps/api/env/.env.functional.test (1)

1-42: Optional: Consider alphabetically ordering environment keys.

The static analysis tool flagged that RPC_NODE_ENDPOINT should be ordered before UAKT_TOP_UP_MASTER_WALLET_MNEMONIC (alphabetically). While this is a minor issue, reordering keys can improve consistency and maintainability. This is optional and can be deferred if not part of your project's style guidelines.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0efaa and 6d7ecbd.

📒 Files selected for processing (12)
  • apps/api/env/.env.functional.test (1 hunks)
  • apps/api/src/background-jobs-app.ts (1 hunks)
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (0 hunks)
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts (0 hunks)
  • apps/api/src/billing/providers/signing-client.provider.ts (0 hunks)
  • apps/api/src/console.ts (0 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts (1 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts (1 hunks)
  • apps/api/src/core/providers/postgres.provider.ts (2 hunks)
  • apps/api/src/core/services/start-server/start-server.ts (0 hunks)
  • apps/api/src/db/dbConnection.ts (2 hunks)
  • apps/api/src/rest-app.ts (1 hunks)
💤 Files with no reviewable changes (5)
  • apps/api/src/billing/providers/signing-client.provider.ts
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts
  • apps/api/src/console.ts
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts
  • apps/api/src/core/services/start-server/start-server.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts
  • apps/api/src/background-jobs-app.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/src/db/dbConnection.ts
  • apps/api/src/core/providers/postgres.provider.ts
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
  • apps/api/src/rest-app.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/src/db/dbConnection.ts
  • apps/api/src/core/providers/postgres.provider.ts
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
  • apps/api/src/rest-app.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
📚 Learning: 2025-08-20T11:41:36.039Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 1740
File: apps/log-collector/src/services/pod-logs-collector/pod-logs-collector.service.ts:127-145
Timestamp: 2025-08-20T11:41:36.039Z
Learning: In log collection services like PodLogsCollectorService, write promises for continuous log streaming are intentionally long-running and expected to remain pending during normal operation. Using Promise.all to wait for both write and streaming promises ensures proper error propagation from either stream, which is the correct pattern for long-running log collection processes.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
🧬 Code graph analysis (3)
apps/api/src/db/dbConnection.ts (3)
apps/api/src/rest-app.ts (2)
  • ON_APP_START (192-194)
  • connectUsingSequelize (198-198)
apps/api/src/app/providers/jobs.provider.ts (1)
  • ON_APP_START (13-23)
apps/api/src/core/providers/app-initializer.ts (1)
  • AppInitializer (6-8)
apps/api/src/core/providers/postgres.provider.ts (2)
apps/api/src/db/dbConnection.ts (2)
  • ON_APP_START (60-62)
  • closeConnections (55-55)
apps/api/src/core/providers/app-initializer.ts (2)
  • ON_APP_START (4-4)
  • AppInitializer (6-8)
apps/api/src/rest-app.ts (1)
apps/api/src/core/providers/postgres.provider.ts (1)
  • migratePG (27-27)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.functional.test

[warning] 5-5: [UnorderedKey] The RPC_NODE_ENDPOINT key should go before the UAKT_TOP_UP_MASTER_WALLET_MNEMONIC key

(UnorderedKey)

⏰ 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). (2)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (8)
apps/api/env/.env.functional.test (1)

5-5: Endpoint URL update is correct.

The RPC endpoint change from sandbox-2 to sandbox aligns with the current test environment configuration and appears to be a valid update.

apps/api/src/rest-app.ts (1)

200-204: LGTM! Disposal now centralized via DisposableRegistry.

The removal of the beforeEnd hook is correct. Resource disposal is now handled automatically by the container calling DisposableRegistry.dispose() at shutdown, which provides centralized cleanup management.

apps/api/src/db/dbConnection.ts (1)

56-66: LGTM! DisposableRegistry integration is correct.

The wrapping of the APP_INITIALIZER factory with DisposableRegistry.registerFromFactory properly registers the database connection cleanup for automatic disposal at shutdown. The type constraint satisfies AppInitializer & Disposable ensures compile-time safety.

apps/api/src/core/providers/postgres.provider.ts (1)

59-67: LGTM! Consistent DisposableRegistry pattern.

The implementation follows the same correct pattern as in dbConnection.ts, ensuring postgres.js clients are properly cleaned up at shutdown through the DisposableRegistry.

apps/api/src/core/lib/disposable-registry/disposable-registry.ts (4)

1-25: LGTM! Clean singleton pattern with delegation.

The static method provides a convenient API while properly delegating to the singleton instance. The use of @singleton() ensures a single registry manages all disposables.


39-49: LGTM! Wrapper correctly registers disposables.

The factory wrapper properly detects disposable return values and registers their cleanup callbacks. The implementation correctly assumes factories are synchronous, which aligns with the current usage patterns in postgres.provider.ts and dbConnection.ts.


62-72: Excellent! Promise.allSettled ensures complete cleanup.

The use of Promise.allSettled correctly addresses the past review concern. All disposal operations will be attempted even if some fail, and any failures are properly surfaced via AggregateError. This guarantees robust resource cleanup at shutdown.


80-82: LGTM! Robust type guard.

The type guard correctly handles edge cases (null, undefined, non-objects) and safely narrows the type to Disposable when all checks pass.

Copy link
Contributor

@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: 2

🧹 Nitpick comments (1)
apps/api/env/.env.functional.test (1)

2-5: Address key ordering inconsistency.

The environment file has inconsistent key ordering. According to static analysis, RPC_NODE_ENDPOINT (line 5) should be ordered alphabetically before UAKT_TOP_UP_MASTER_WALLET_MNEMONIC (line 2). Consider reorganizing keys alphabetically for consistency and maintainability.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d7ecbd and 8bd14e0.

📒 Files selected for processing (13)
  • apps/api/env/.env.functional.test (1 hunks)
  • apps/api/src/background-jobs-app.ts (1 hunks)
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (0 hunks)
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts (0 hunks)
  • apps/api/src/billing/providers/signing-client.provider.ts (0 hunks)
  • apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts (0 hunks)
  • apps/api/src/console.ts (0 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts (1 hunks)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts (1 hunks)
  • apps/api/src/core/providers/postgres.provider.ts (2 hunks)
  • apps/api/src/core/services/start-server/start-server.ts (0 hunks)
  • apps/api/src/db/dbConnection.ts (2 hunks)
  • apps/api/src/rest-app.ts (1 hunks)
💤 Files with no reviewable changes (6)
  • apps/api/src/billing/providers/signing-client.provider.ts
  • apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts
  • apps/api/src/console.ts
  • apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts
  • apps/api/src/core/services/start-server/start-server.ts
  • apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/api/src/core/lib/disposable-registry/disposable-registry.spec.ts
  • apps/api/src/db/dbConnection.ts
  • apps/api/src/core/providers/postgres.provider.ts
  • apps/api/src/background-jobs-app.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/src/rest-app.ts
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/src/rest-app.ts
  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.

Applied to files:

  • apps/api/src/core/lib/disposable-registry/disposable-registry.ts
🧬 Code graph analysis (1)
apps/api/src/rest-app.ts (1)
apps/api/src/core/providers/postgres.provider.ts (1)
  • migratePG (27-27)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.functional.test

[warning] 5-5: [UnorderedKey] The RPC_NODE_ENDPOINT key should go before the UAKT_TOP_UP_MASTER_WALLET_MNEMONIC key

(UnorderedKey)

⏰ 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). (2)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (3)
apps/api/env/.env.functional.test (1)

5-5: Verify the new RPC endpoint is correct and accessible.

The RPC endpoint has been updated from https://rpc.sandbox-2.aksh.pw:443 to https://rpc.sandbox.akt.dev/rpc. Ensure that the new endpoint is valid, accessible, and appropriate for the sandbox environment used by functional tests.

apps/api/src/core/lib/disposable-registry/disposable-registry.ts (2)

80-82: LGTM!

The type guard implementation is solid and correctly identifies objects that implement the Disposable interface. The truthy check, object type check, and function verification provide proper type safety.


39-49: ****

tsyringe's FactoryProvider useFactory signature is synchronous: (dependencyContainer: DependencyContainer) => T, and does not natively support async factories. The review comment's concern is invalid for this codebase.

The proposed diff—making the wrapper async—would break the type contract because it would return Promise<T> instead of T, violating tsyringe's expectations for factory registration. Earlier script results confirmed both factories in the codebase (dbConnection.ts and postgres.provider.ts) are synchronous; they return objects with async methods, not Promises from the factory itself.

If async factories were needed, the codebase would require manual async setup before container use, or use @launchtray/tsyringe-async.

No action is required; the code is correct as-is.

Likely an incorrect or invalid review comment.

@stalniy
Copy link
Contributor

stalniy commented Nov 4, 2025

microsoft/tsyringe#274

@ygrishajev ygrishajev merged commit 23d3c1e into main Nov 4, 2025
62 checks passed
@ygrishajev ygrishajev deleted the feature/di branch November 4, 2025 10:47
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

Comments