feat: Forgot to expose noOfIsolates on serve extension on RelicApp #260
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdded Changes
Sequence DiagramsequenceDiagram
participant Test as test/router
participant Serve as RelicAppIOServeEx.serve()
participant Run as run()
participant IOAdapter as IOAdapter.bind()
participant MultiIsolate as _MultiIsolateRelicServer
Test->>Serve: serve(v6Only, noOfIsolates=2, shared=false)
Serve->>Serve: shared = false || (2 > 1) = true
Serve->>IOAdapter: bind(v6Only, shared=true)
Serve->>Run: run(noOfIsolates=2)
Run->>MultiIsolate: constructor(noOfIsolates=2)
MultiIsolate->>MultiIsolate: assert(2 > 1) ✓
MultiIsolate->>MultiIsolate: _children = List.generate(2, ...)
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly Related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
==========================================
+ Coverage 91.48% 91.50% +0.01%
==========================================
Files 89 89
Lines 3475 3483 +8
Branches 1769 1771 +2
==========================================
+ Hits 3179 3187 +8
Misses 296 296 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
2 similar comments
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/src/adapter/io/io_serve.dart (1)
17-25: Document the new parameters.The
v6OnlyandnoOfIsolatesparameters are not documented in the method docstring. Consider adding documentation explaining:
v6Only: Controls whether an IPv6 socket should only accept IPv6 connections (when using IPv6 addresses)noOfIsolates: Number of isolates to spawn for handling requests (defaults to 1 for single-isolate mode)Apply this diff to add parameter documentation:
extension RelicAppIOServeEx on RelicApp { /// Starts a [HttpServer] that listens on the specified [address] and /// [port] and sends requests to [handler]. /// /// If [securityContext] is provided, a secure HTTPS server will be started /// using [HttpServer.bindSecure]. Otherwise, an HTTP server will be started /// using [HttpServer.bind]. /// /// If not specified [address] will default to [InternetAddress.loopbackIPv4], /// and [port] to 8080. + /// + /// The [v6Only] parameter controls whether an IPv6 socket accepts only IPv6 + /// connections (ignored for IPv4 addresses). + /// + /// The [noOfIsolates] parameter specifies the number of isolates to spawn + /// for handling requests. When greater than 1, [shared] is automatically + /// enabled to allow multiple isolates to bind to the same port. Future<RelicServer> serve({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/src/adapter/io/io_serve.dart(1 hunks)lib/src/relic_server.dart(1 hunks)test/router/relic_app_test.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: All Dart code must pass static analysis usingdart analyze --fatal-infoswith no issues
All Dart files must be formatted withdart format(CI enforcesdart format --set-exit-if-changed .)
Files:
test/router/relic_app_test.dartlib/src/adapter/io/io_serve.dartlib/src/relic_server.dart
test/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
test/**/*.dart: Tests should follow the Given-When-Then pattern in descriptions (flexible structuring allowed)
Use Arrange-Act-Assert pattern within test bodies
Provide clear, descriptive test titles; prefer single responsibility per test unless related assertions improve clarity
Place tests in thetest/directory mirroring thelib/structure
Files:
test/router/relic_app_test.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
lib/**/*.dart: Use Uint8List for request/response bodies for performance; avoid List for body payloads
Use type-safe HTTP header parsing and validation when accessing headers
Use router with trie-based matching and symbol-based path parameters (e.g.,#name,#age) for routing
Ensure WebSocket handling includes proper lifecycle management (e.g., ping/pong for connection health)
Files:
lib/src/adapter/io/io_serve.dartlib/src/relic_server.dart
🧠 Learnings (6)
📓 Common learnings
Learnt from: nielsenko
Repo: serverpod/relic PR: 216
File: lib/src/router/relic_app.dart:47-49
Timestamp: 2025-10-22T11:25:39.264Z
Learning: In the serverpod/relic repository, validation of the `noOfIsolates` parameter should be handled in the `RelicServer` constructor (lib/src/relic_server.dart), not in `RelicApp.run` (lib/src/router/relic_app.dart).
Learnt from: nielsenko
Repo: serverpod/relic PR: 212
File: example/multi_isolate.dart:32-38
Timestamp: 2025-10-16T07:49:04.663Z
Learning: In the Relic framework, the default binding for RelicApp.serve() is intentionally set to InternetAddress.loopbackIPv4 (localhost only) rather than anyIPv4. This is a deliberate security-first design decision to require explicit consent (via the address parameter) before exposing endpoints outside the host.
Learnt from: nielsenko
Repo: serverpod/relic PR: 48
File: lib/src/handler/handler.dart:59-67
Timestamp: 2025-04-25T07:39:38.915Z
Learning: Nielsenko prefers using switch statements with pattern matching over if statements when working with sealed classes in Dart, as they provide exhaustiveness checking at compile time and can be more concise.
📚 Learning: 2025-10-22T11:25:39.264Z
Learnt from: nielsenko
Repo: serverpod/relic PR: 216
File: lib/src/router/relic_app.dart:47-49
Timestamp: 2025-10-22T11:25:39.264Z
Learning: In the serverpod/relic repository, validation of the `noOfIsolates` parameter should be handled in the `RelicServer` constructor (lib/src/relic_server.dart), not in `RelicApp.run` (lib/src/router/relic_app.dart).
Applied to files:
test/router/relic_app_test.dartlib/src/adapter/io/io_serve.dartlib/src/relic_server.dart
📚 Learning: 2025-10-09T16:21:09.310Z
Learnt from: CR
Repo: serverpod/relic PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T16:21:09.310Z
Learning: Applies to test/**/*.dart : Provide clear, descriptive test titles; prefer single responsibility per test unless related assertions improve clarity
Applied to files:
test/router/relic_app_test.dart
📚 Learning: 2025-10-20T14:05:34.503Z
Learnt from: nielsenko
Repo: serverpod/relic PR: 226
File: lib/src/router/relic_app.dart:147-156
Timestamp: 2025-10-20T14:05:34.503Z
Learning: In the relic codebase, hot-reload and other development-time features in lib/src/router/relic_app.dart should fail loudly (raise exceptions) rather than catch and suppress errors, as this helps developers diagnose issues during development.
Applied to files:
test/router/relic_app_test.dart
📚 Learning: 2025-04-24T14:06:32.810Z
Learnt from: nielsenko
Repo: serverpod/relic PR: 48
File: example/example.dart:31-36
Timestamp: 2025-04-24T14:06:32.810Z
Learning: In the example code, `sleep()` is intentionally used instead of `await Future.delayed()` to simulate CPU-bound work that benefits from multiple isolates/cores. Using a blocking call demonstrates why multiple isolates are necessary, while an async approach would allow a single isolate to handle multiple requests concurrently, defeating the purpose of the multi-isolate example.
Applied to files:
test/router/relic_app_test.dart
📚 Learning: 2025-10-16T07:49:04.663Z
Learnt from: nielsenko
Repo: serverpod/relic PR: 212
File: example/multi_isolate.dart:32-38
Timestamp: 2025-10-16T07:49:04.663Z
Learning: In the Relic framework, the default binding for RelicApp.serve() is intentionally set to InternetAddress.loopbackIPv4 (localhost only) rather than anyIPv4. This is a deliberate security-first design decision to require explicit consent (via the address parameter) before exposing endpoints outside the host.
Applied to files:
lib/src/adapter/io/io_serve.dart
🔇 Additional comments (3)
test/router/relic_app_test.dart (1)
95-95: LGTM! Multi-isolate hot-reload testing.The addition of
noOfIsolates: 2appropriately exercises the newly exposed parameter and validates that hot-reload works correctly with multiple isolates.lib/src/relic_server.dart (1)
233-234: LGTM! Defensive assertion for multi-isolate server.The assertion ensures
_MultiIsolateRelicServeris only instantiated whennoOfIsolates > 1, catching any logic errors if this constructor were called directly instead of through the factory.lib/src/adapter/io/io_serve.dart (1)
33-33: LGTM! Automatic shared mode for multi-isolate.The logic
shared || noOfIsolates > 1correctly ensures thatsharedis enabled when using multiple isolates (required for port sharing). This overrides any explicitshared: falsethe user might pass whennoOfIsolates > 1.
SandPod
left a comment
SandPod
left a comment
There was a problem hiding this comment.
Two minor comments, otherwise LGTM! 👍
2 participants
Description
Exposes the
noOfIsolatesparameter onRelicApp.serve()to allow multi-isolate servers. Previously,noOfIsolateswas only available when constructing aRelicServerdirectly, or viaRelicApp.run, but not through theRelicApp.serve()extension method.Changes:
noOfIsolatesparameter toRelicAppIOServeEx.serve()extension methodv6Onlyparameter toRelicAppIOServeEx.serve()sharedwhennoOfIsolates > 1(required for multi-isolate binding)noOfIsolatesthrough torun()method_MultiIsolateRelicServerconstructor to ensurenoOfIsolates > 1noOfIsolates: 2Related Issues
Pre-Launch Checklist
///), ensuring consistency with existing project documentation.Breaking Changes
Additional Notes
None.
Summary by CodeRabbit