feature: implement new dispatchers - #63
Conversation
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds dispatcher-mode serving with bounded request intake, PHP HTTP value objects, request and response exchange handling, timer control, worker lifecycle cleanup, workspace metadata, PHP build support, and comprehensive integration tests. ChangesDispatcher runtime and PHP HTTP integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RapiraHandle
participant IntakeChannel
participant Dispatcher
participant Exchange
participant Worker
Client->>RapiraHandle: submit request
RapiraHandle->>IntakeChannel: enqueue timestamped job
Dispatcher->>IntakeChannel: receive job
Dispatcher->>Exchange: create exchange
Exchange->>Worker: expose request and response operations
Worker->>Exchange: write head and body
Exchange->>Client: emit finalized response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the PHP-facing surface of Rapira by adding missing __construct definitions to HTTP value types and tightening “host-created only” construction semantics, with accompanying regenerated stub/arginfo outputs and new PHP fixtures to exercise the behavior.
Changes:
- Add missing
__constructsignatures to HTTP value objects (e.g.,InetAddress,Request,Multipart) and regenerate corresponding arginfo. - Mark internal HTTP classes as host-created via private constructors in stubs (and add fixtures that assert construction/cloning behavior).
- Update
.clang-tidyto use a curated check set instead of the previous default.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/tests/fixtures/http_values/construct.php | New fixture exercising construction/readonly/type/arity constraints for HTTP value objects. |
| crates/tests/fixtures/dispatcher/worker-singleton.php | New fixture checking dispatcher singleton/cloning behavior and logging properties. |
| crates/tests/fixtures/dispatcher/host-created-only.php | New fixture asserting internal dispatcher is not user-constructible. |
| crates/php_sys/rapira.stub.php | Clarifies Dispatcher interface doc comment about plugin narrowing. |
| crates/php_sys/rapira_http.stub.php | Adds missing constructors for HTTP value types; marks internal HTTP classes host-created via private constructors. |
| crates/php_sys/rapira_http_arginfo.h | Regenerated arginfo to reflect new constructors and method tables. |
| crates/php_sys/rapira_arginfo.h | Regenerated stub hash metadata. |
| .clang-tidy | Switches to curated clang-tidy checks list (and updates formatting). |
Suppressed comments (1)
.clang-tidy:9
- This PR description focuses on adding missing PHP __construct methods / regenerating stubs, but it also changes the default clang-tidy check set (from the previous survey-mode default to a curated list). If intentional, it should be called out in the PR description (or split into a separate PR) because it can change local/CI lint behavior.
Checks: >
bugprone-*,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (5)
crates/php_sys/wrapper.h:30
boolrequires a visible definition (typically via<stdbool.h>). Since this header is reused across translation units, add#include <stdbool.h>before usingbool(or switch the declaration to a PHP/Zend type likezend_bool) to avoid compilation failures on toolchains where PHP headers don't providebool.
extern bool rapira_worker_mode;
sapi_globals_struct *rapira_sg(void);
crates/php_sys/rapira_dispatcher.c:184
- These internal constructors throw but don’t
RETURN_THROWS(). AddRETURN_THROWS();afterzend_throw_error(...)(or otherwise immediately return in the same pattern used elsewhere in this file) so the engine always sees a clear “exceptional return” path for these internal methods.
ZEND_METHOD(Rapira_Internal_Http_Dispatcher, __construct) {
zend_throw_error(NULL, "host-created; obtain it from \\Rapira\\get_dispatcher()");
}
ZEND_METHOD(Rapira_Internal_Http_DispatcherInfo, __construct) {
zend_throw_error(NULL, "host-created");
}
ZEND_METHOD(Rapira_Internal_Http_Exchange, __construct) {
zend_throw_error(NULL, "host-created");
}
crates/php_sys/rapira_dispatcher.c:199
- The new "not implemented" errors are very generic and will be hard to debug in userland (especially now that
get_dispatcher()can succeed). Consider including the concrete method name and actionable guidance in the message (e.g., which mode/feature gate is required, or that Exchange verbs are not available yet) so failures are self-explanatory.
ZEND_METHOD(Rapira_Internal_Http_Dispatcher, tryReceive) {
zend_throw_error(NULL, "not implemented");
RETURN_THROWS();
}
ZEND_METHOD(Rapira_Internal_Http_Dispatcher, receive) {
zend_throw_error(NULL, "not implemented");
RETURN_THROWS();
}
ZEND_METHOD(Rapira_Internal_Http_Dispatcher, getInfo) {
zend_throw_error(NULL, "not implemented");
RETURN_THROWS();
}
crates/php_sys/src/start.rs:66
- The previous doc comment here explained important lifecycle/teardown semantics for worker-side start. Replacing it with a terse comment loses key context for maintainers; please restore a concise doc comment describing the contract (module startup/teardown expectations, thread lifecycle, and how it differs from fused
start).
// worker side part
pub fn start_worker(mode: Mode, hooks: WorkerHooks) -> anyhow::Result<Self> {
let WorkerHooks {
crates/php_sys/src/handler.rs:15
- The PR removes the API-level documentation for
RapiraHandle(shutdown contract and lifecycle expectations). Since this is a public type, restoring a shorter version of that doc (or moving it into module-level docs) would preserve important usage guarantees without bloating the file.
#[derive(Clone)]
pub struct RapiraHandle {
intake: mpsc::Sender<Job>,
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
rustatian
changed the title
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main.rs (1)
135-141: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale comment: the non-classic branch now selects
Dispatcher.The comment names
Workeras the non-classic mode, but line 140 constructsMode::Dispatcher. The two statements now contradict each other.📝 Proposed comment correction
- // Both forms run the same worker model; --classic only changes whether the script is - // re-included per request (Classic) or stays resident (Worker). + // Both forms run the same worker model; --classic only changes whether the script is + // re-included per request (Classic) or stays resident and pulls units from the + // dispatcher (Dispatcher). let mode = if settings.pool.classic { Mode::Classic } else { Mode::Dispatcher(script.clone()) };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.rs` around lines 135 - 141, Update the comment above the mode selection to describe the non-classic branch as Dispatcher rather than Worker, while preserving the explanation that --classic controls whether the script is re-included per request or remains resident.crates/php_sys/src/types.rs (1)
139-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the invariant documentation on
ContextandStreamState.This PR removes the doc comments that recorded the
StreamStateprogression, its invariant, theContextfield roles, theis_truncatedrule, and the sealing contract offinish. The same PR adds a second writer to that state:crates/php_sys/src/exchange.rssetsctx.headandctx.stream = StreamState::HeadSentdirectly (lines 361-362 and 386-390) and deliberately bypassesContext::commit_head. It also relies onfinishsending exactly oneFrameand dropping the sender.Two independent code paths now depend on these unwritten rules. Restore the invariants as doc comments on
StreamState,Context,is_truncated, andfinish, so the classic path and the Exchange path share one written contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/php_sys/src/types.rs` around lines 139 - 147, Restore documentation comments for the StreamState progression and invariant, Context field roles, the is_truncated rule, and finish’s sealing contract. Anchor the comments to the StreamState definition, Context struct, is_truncated, and finish, explicitly covering direct head/stream updates in exchange.rs and that finish sends exactly one Frame before dropping the sender.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/php_sys/rapira_exchange.c`:
- Around line 232-246: Update the header-building loop around
rapira_rs_exchange_header to preserve repeated names: look up an existing list
in headers and append value.ptr/value.len to it, otherwise create and insert a
new list. Keep zend_symtable key handling so digit-only names retain integer-key
behavior, and stop replacing previously accumulated values.
In `@crates/php_sys/rapira_http.c`:
- Around line 195-250: Update the TLS assignment in the request-construction
function around zend_update_property(..., "tls") to explicitly handle tls ==
NULL, storing a PHP null value instead of passing the null C pointer to the
property writer. Preserve the existing object assignment for non-null TLS
objects and retain exception checking after the property update.
In `@crates/php_sys/src/exchange.rs`:
- Around line 71-78: Update reclaim_current to emit
sb_update(Event::Handled(true)) and perform the associated quota::tick() when
reclaiming an unfinalized Unit::Handling pointer, while preserving the existing
drop behavior. Do not charge Unit::Sealed; distinguish the two variants so only
the unfinalized handling unit updates the scoreboard before being reclaimed.
In `@crates/php_sys/src/handler.rs`:
- Around line 55-92: Replace the Full branch in handle with cancellable async
admission or semaphore-based queueing; remove the per-request spawn_blocking
retry loop, deadline, and sleep. Preserve bounded backpressure and return the
existing worker-pool-stopped-or-saturated error when admission fails or the
worker pool disconnects.
In `@crates/php_sys/src/rapira_worker.rs`:
- Around line 98-117: The dispatcher path must emit exactly one
scoreboard::Event::Recycled whenever it returns Cycle::Recycle, and emit
scoreboard::Event::Healthy when served_any() is true before returning
Cycle::Restart after a shutdown bailout. Update the dispatcher cycle-handling
logic around the visible Cycle::Stop/Cycle::Recycle/Cycle::Failed branches,
preserving existing Healthy behavior and ensuring the events are emitted on
every applicable return path.
In `@crates/php_sys/src/start.rs`:
- Around line 93-99: Replace the joke SAFETY comment above the assignment to
crate::rapira_mode in start_worker with a precise invariant: start_worker is
called only once per process, and rapira_mode is fully initialized before the
spawned worker thread or C code can read it. Preserve the existing write and
unsafe block unless converting the value to an atomic is necessary.
In `@crates/tests/tests/timeout_tests.rs`:
- Around line 107-116: Remove the #[ignore] attribute from
max_execution_time_fires_on_rearmed_jobs so the dispatcher-based timeout test
runs in CI. Keep the existing Mode::Dispatcher setup and test behavior
unchanged.
- Around line 36-42: Update the timeout test around the loop over "/first" and
"/second" so it verifies the worker has entered its receive state before the
two-second sleep, using a readiness signal or a completed warm-up exchange.
Ensure the timed idle intervals begin only after that synchronization, while
preserving the existing drain and per-unit timer assertions.
---
Outside diff comments:
In `@crates/php_sys/src/types.rs`:
- Around line 139-147: Restore documentation comments for the StreamState
progression and invariant, Context field roles, the is_truncated rule, and
finish’s sealing contract. Anchor the comments to the StreamState definition,
Context struct, is_truncated, and finish, explicitly covering direct head/stream
updates in exchange.rs and that finish sends exactly one Frame before dropping
the sender.
In `@src/main.rs`:
- Around line 135-141: Update the comment above the mode selection to describe
the non-classic branch as Dispatcher rather than Worker, while preserving the
explanation that --classic controls whether the script is re-included per
request or remains resident.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 25308c16-6c0c-44c0-83c5-f37a166851a7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (53)
.clang-tidyCargo.tomlMakefilecrates/api/Cargo.tomlcrates/config/Cargo.tomlcrates/master/Cargo.tomlcrates/php_sys/Cargo.tomlcrates/php_sys/allowed_bindings.rscrates/php_sys/build.rscrates/php_sys/module.ccrates/php_sys/rapira.stub.phpcrates/php_sys/rapira_arginfo.hcrates/php_sys/rapira_classes.ccrates/php_sys/rapira_classes.hcrates/php_sys/rapira_dispatcher.ccrates/php_sys/rapira_exchange.ccrates/php_sys/rapira_http.ccrates/php_sys/rapira_http.stub.phpcrates/php_sys/rapira_http_arginfo.hcrates/php_sys/src/callbacks.rscrates/php_sys/src/exchange.rscrates/php_sys/src/handler.rscrates/php_sys/src/lib.rscrates/php_sys/src/rapira_worker.rscrates/php_sys/src/scoreboard.rscrates/php_sys/src/start.rscrates/php_sys/src/types.rscrates/php_sys/wrapper.hcrates/runtime/Cargo.tomlcrates/runtime/src/lib.rscrates/scoreboard/Cargo.tomlcrates/tests/fixtures/dispatcher/echo-loop-worker.phpcrates/tests/fixtures/dispatcher/host-created-only.phpcrates/tests/fixtures/dispatcher/recv-probes-worker.phpcrates/tests/fixtures/dispatcher/request-worker.phpcrates/tests/fixtures/dispatcher/verbs-worker.phpcrates/tests/fixtures/dispatcher/worker-singleton.phpcrates/tests/fixtures/http_values/construct.phpcrates/tests/src/lib.rscrates/tests/tests/async_tests.rscrates/tests/tests/basic_tests.rscrates/tests/tests/dispatcher.rscrates/tests/tests/dispatcher_loop.rscrates/tests/tests/extension_tests.rscrates/tests/tests/failboot_worker_tests.rscrates/tests/tests/general_tests.rscrates/tests/tests/http_values.rscrates/tests/tests/observer_teardown_tests.rscrates/tests/tests/observer_tests.rscrates/tests/tests/php_ext_tests.rscrates/tests/tests/ported_tests.rscrates/tests/tests/timeout_tests.rssrc/main.rs
💤 Files with no reviewable changes (1)
- crates/php_sys/src/scoreboard.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: test (ubuntu-latest, 8.5)
🧰 Additional context used
🪛 checkmake (0.3.2)
Makefile
[warning] 75-75: Target body for "php" exceeds allowed length of 5 lines (13).
(maxbodylength)
🪛 Cppcheck (2.21.0)
crates/php_sys/rapira_classes.c
[error] 7-7: syntax error
(syntaxError)
crates/php_sys/module.c
[style] 85-85: The function 'rapira_exchange_from' is never used.
(unusedFunction)
[style] 93-93: The function 'rapira_dispatcher_info_from' is never used.
(unusedFunction)
[style] 79-79: The function 'PHP_RSHUTDOWN_FUNCTION' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
[style] 162-162: The function 'rapira_receive_untimed' is never used.
(unusedFunction)
[style] 176-176: The function 'rapira_receive_timed' is never used.
(unusedFunction)
crates/php_sys/rapira_exchange.c
[error] 78-78: There is an unknown macro here somewhere. Configuration is required. If ZEND_PARSE_PARAMETERS_START is a macro then please configure it.
(unknownMacro)
crates/php_sys/rapira_http.c
[error] 32-32: There is an unknown macro here somewhere. Configuration is required. If ZEND_PARSE_PARAMETERS_START is a macro then please configure it.
(unknownMacro)
🪛 PHPMD (2.15.0)
crates/php_sys/rapira.stub.php
[warning] 75-75: Avoid variables with short names like $ip. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 78-78: Avoid variables with short names like $ip. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 78-78: Avoid unused parameters such as '$ip'. (undefined)
(UnusedFormalParameter)
[warning] 78-78: Avoid unused parameters such as '$port'. (undefined)
(UnusedFormalParameter)
[warning] 91-91: Avoid unused parameters such as '$path'. (undefined)
(UnusedFormalParameter)
crates/php_sys/rapira_http.stub.php
[warning] 24-24: Avoid unused parameters such as '$version'. (undefined)
(UnusedFormalParameter)
[warning] 25-25: Avoid unused parameters such as '$cipher'. (undefined)
(UnusedFormalParameter)
[warning] 26-26: Avoid unused parameters such as '$negotiatedProtocol'. (undefined)
(UnusedFormalParameter)
[warning] 27-27: Avoid unused parameters such as '$requestedServerName'. (undefined)
(UnusedFormalParameter)
[warning] 28-28: Avoid unused parameters such as '$certSerial'. (undefined)
(UnusedFormalParameter)
[warning] 29-29: Avoid unused parameters such as '$certOrganization'. (undefined)
(UnusedFormalParameter)
[warning] 30-30: Avoid unused parameters such as '$certFingerprint'. (undefined)
(UnusedFormalParameter)
[warning] 48-48: Avoid unused parameters such as '$name'. (undefined)
(UnusedFormalParameter)
[warning] 48-48: Avoid unused parameters such as '$value'. (undefined)
(UnusedFormalParameter)
[warning] 48-48: Avoid unused parameters such as '$headers'. (undefined)
(UnusedFormalParameter)
[warning] 69-69: Avoid unused parameters such as '$name'. (undefined)
(UnusedFormalParameter)
[warning] 70-70: Avoid unused parameters such as '$clientFilename'. (undefined)
(UnusedFormalParameter)
[warning] 71-71: Avoid unused parameters such as '$clientMediaType'. (undefined)
(UnusedFormalParameter)
[warning] 72-72: Avoid unused parameters such as '$headers'. (undefined)
(UnusedFormalParameter)
[warning] 73-73: Avoid unused parameters such as '$tmpPath'. (undefined)
(UnusedFormalParameter)
[warning] 74-74: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
[warning] 91-91: Avoid unused parameters such as '$fields'. (undefined)
(UnusedFormalParameter)
[warning] 91-91: Avoid unused parameters such as '$files'. (undefined)
(UnusedFormalParameter)
[warning] 115-127: The method __construct has 11 parameters. Consider reducing the number of parameters to less than 10. (undefined)
(ExcessiveParameterList)
[warning] 116-116: Avoid unused parameters such as '$method'. (undefined)
(UnusedFormalParameter)
[warning] 117-117: Avoid unused parameters such as '$uri'. (undefined)
(UnusedFormalParameter)
[warning] 118-118: Avoid unused parameters such as '$target'. (undefined)
(UnusedFormalParameter)
[warning] 119-119: Avoid unused parameters such as '$authority'. (undefined)
(UnusedFormalParameter)
[warning] 120-120: Avoid unused parameters such as '$protocol'. (undefined)
(UnusedFormalParameter)
[warning] 121-121: Avoid unused parameters such as '$headers'. (undefined)
(UnusedFormalParameter)
[warning] 122-122: Avoid unused parameters such as '$body'. (undefined)
(UnusedFormalParameter)
[warning] 123-123: Avoid unused parameters such as '$remote'. (undefined)
(UnusedFormalParameter)
[warning] 124-124: Avoid unused parameters such as '$server'. (undefined)
(UnusedFormalParameter)
[warning] 125-125: Avoid unused parameters such as '$tls'. (undefined)
(UnusedFormalParameter)
[warning] 126-126: Avoid unused parameters such as '$receivedAt'. (undefined)
(UnusedFormalParameter)
🪛 PHPStan (2.2.7)
crates/tests/fixtures/dispatcher/echo-loop-worker.php
[warning] 5-5: While loop condition is always true.
(while.alwaysTrue)
crates/tests/fixtures/dispatcher/host-created-only.php
[warning] 4-4: Call to new Rapira\Internal\Http\Dispatcher() on a separate line has no effect.
(new.resultUnused)
[warning] 4-4: Cannot instantiate class Rapira\Internal\Http\Dispatcher via private constructor Rapira\Internal\Http\Dispatcher::__construct().
(new.privateConstructor)
crates/tests/fixtures/dispatcher/worker-singleton.php
[warning] 6-6: Expression "clone $d" on a separate line does not do anything.
(expr.resultUnused)
[warning] 8-8: Dead catch - Error is never thrown in the try block.
(catch.neverThrown)
[warning] 19-19: Instanceof between Rapira\Dispatcher and Rapira\Dispatcher will always evaluate to true.
If Rapira\Dispatcher::name() is impure, add @phpstan-impure PHPDoc tag above its declaration. Learn more: https://phpstan.org/blog/remembering-and-forgetting-returned-values
(instanceof.alwaysTrue)
crates/tests/fixtures/http_values/construct.php
[warning] 35-35: Cannot access property $fields on Rapira\Http\Multipart|string.
(property.nonObject)
[warning] 35-35: Cannot access property $fields on Rapira\Http\Multipart|string.
(property.nonObject)
[warning] 36-36: Cannot access property $files on Rapira\Http\Multipart|string.
(property.nonObject)
[warning] 36-36: Cannot access property $files on Rapira\Http\Multipart|string.
(property.nonObject)
[warning] 37-37: Cannot access property $certSerial on Rapira\Http\Tls|null.
(property.nonObject)
[warning] 37-37: Cannot access property $negotiatedProtocol on Rapira\Http\Tls|null.
(property.nonObject)
[warning] 43-43: Readonly property Rapira\Http\Request::$method is assigned outside of its declaring class.
(property.readOnlyAssignOutOfClass)
[warning] 45-45: Dead catch - Error is never thrown in the try block.
(catch.neverThrown)
[warning] 50-50: Call to new Rapira\Http\Request() on a separate line has no effect.
(new.resultUnused)
[warning] 50-50: Class Rapira\Http\Request constructor invoked with 1 parameter, 11 required.
(arguments.count)
[warning] 57-57: Call to new Rapira\Http\Request() on a separate line has no effect.
(new.resultUnused)
[warning] 57-57: Parameter #8 $remote of class Rapira\Http\Request constructor expects Rapira\InetAddress|Rapira\UnixAddress, string given.
(argument.type)
crates/tests/fixtures/dispatcher/request-worker.php
[warning] 7-7: While loop condition is always true.
(while.alwaysTrue)
crates/tests/fixtures/dispatcher/verbs-worker.php
[warning] 39-39: Dead catch - Error is never thrown in the try block.
(catch.neverThrown)
[warning] 132-132: While loop condition is always true.
(while.alwaysTrue)
crates/php_sys/rapira.stub.php
[warning] 75-75: Class Rapira\InetAddress has an uninitialized readonly property $ip. Assign it in the constructor.
(property.uninitializedReadonly)
[warning] 76-76: Class Rapira\InetAddress has an uninitialized readonly property $port. Assign it in the constructor.
(property.uninitializedReadonly)
[warning] 89-89: Class Rapira\UnixAddress has an uninitialized readonly property $path. Assign it in the constructor.
(property.uninitializedReadonly)
crates/php_sys/rapira_http.stub.php
[warning] 110-110: Class Rapira\Http\Request has an uninitialized readonly property $remote. Assign it in the constructor.
(property.uninitializedReadonly)
[warning] 111-111: Class Rapira\Http\Request has an uninitialized readonly property $server. Assign it in the constructor.
(property.uninitializedReadonly)
[warning] 112-112: Class Rapira\Http\Request has an uninitialized readonly property $tls. Assign it in the constructor.
(property.uninitializedReadonly)
[warning] 113-113: Class Rapira\Http\Request has an uninitialized readonly property $receivedAt. Assign it in the constructor.
(property.uninitializedReadonly)
🔇 Additional comments (55)
.clang-tidy (1)
2-20: LGTM!Cargo.toml (1)
3-4: LGTM!Also applies to: 31-33
Makefile (1)
18-18: LGTM!Also applies to: 65-88
crates/api/Cargo.toml (1)
3-4: LGTM!crates/config/Cargo.toml (1)
3-4: LGTM!crates/master/Cargo.toml (1)
3-4: LGTM!crates/php_sys/Cargo.toml (1)
3-4: LGTM!Also applies to: 13-13
crates/php_sys/allowed_bindings.rs (1)
13-16: LGTM!crates/php_sys/build.rs (1)
41-53: LGTM!Also applies to: 89-97
crates/php_sys/module.c (1)
1-9: LGTM!Also applies to: 79-93, 156-182, 551-559
crates/php_sys/src/lib.rs (1)
9-9: LGTM!Also applies to: 47-51
crates/php_sys/wrapper.h (2)
38-64: LGTM!Also applies to: 89-91
66-87: 🎯 Functional CorrectnessKeep the typedef declarations.
crates/php_sys/wrapper.hcontains one declaration of each type. No removal is required.> Likely an incorrect or invalid review comment.crates/tests/fixtures/dispatcher/echo-loop-worker.php (1)
1-16: LGTM!crates/tests/fixtures/dispatcher/host-created-only.php (1)
1-10: LGTM!crates/tests/fixtures/dispatcher/recv-probes-worker.php (1)
1-25: LGTM!crates/tests/fixtures/dispatcher/request-worker.php (1)
1-29: LGTM!crates/tests/fixtures/dispatcher/verbs-worker.php (1)
1-139: LGTM!crates/tests/fixtures/dispatcher/worker-singleton.php (1)
1-21: LGTM!crates/tests/src/lib.rs (1)
57-57: LGTM!crates/tests/tests/async_tests.rs (1)
8-8: LGTM!Also applies to: 24-24, 50-50, 97-97, 119-121, 172-172, 215-215, 252-252, 300-302, 352-352, 366-366, 387-387
crates/tests/tests/basic_tests.rs (1)
46-46: LGTM!Also applies to: 62-62, 88-88, 123-123, 145-147, 187-187, 225-225, 262-262, 310-312, 370-370, 400-400, 421-423, 477-477, 511-513, 545-547, 587-589, 616-618, 661-661, 698-700, 715-717, 741-743, 772-774, 798-800, 814-816, 860-862
crates/tests/tests/php_ext_tests.rs (1)
9-9: LGTM!crates/tests/fixtures/http_values/construct.php (1)
1-64: LGTM!crates/tests/tests/dispatcher.rs (1)
4-4: LGTM!Also applies to: 34-66, 68-84
crates/tests/tests/dispatcher_loop.rs (1)
1-415: LGTM!crates/tests/tests/extension_tests.rs (1)
77-79: LGTM!Also applies to: 154-156, 212-214, 314-316
crates/tests/tests/failboot_worker_tests.rs (1)
20-22: LGTM!Also applies to: 50-52
crates/tests/tests/general_tests.rs (1)
33-33: LGTM!Also applies to: 64-64, 94-96, 130-130, 157-159, 190-192, 220-222, 249-251, 271-273, 294-296, 336-338, 357-359
crates/tests/tests/http_values.rs (1)
1-35: LGTM!crates/tests/tests/observer_teardown_tests.rs (1)
23-25: LGTM!crates/tests/tests/observer_tests.rs (1)
22-24: LGTM!crates/tests/tests/ported_tests.rs (1)
84-86: LGTM!Also applies to: 148-150, 178-180, 220-220, 320-320, 330-332, 367-369, 409-409, 488-490, 510-510, 571-573, 646-646, 666-666, 701-703, 731-733, 755-757, 844-844, 870-872
crates/php_sys/rapira.stub.php (1)
42-45: LGTM!Also applies to: 67-93
crates/php_sys/rapira_arginfo.h (1)
40-54: LGTM!Also applies to: 84-93, 140-173
crates/php_sys/rapira_classes.h (1)
6-11: LGTM!Also applies to: 27-51, 53-70, 72-80, 82-99
crates/php_sys/rapira_exchange.c (1)
21-27: LGTM!Also applies to: 30-51, 56-73, 75-147, 149-198, 202-216, 248-283
crates/php_sys/rapira_http.c (1)
7-27: LGTM!Also applies to: 29-54, 56-101, 103-123, 125-162, 164-178, 180-194
crates/php_sys/rapira_http_arginfo.h (1)
2-46: LGTM!Also applies to: 85-87, 97-105, 123-136, 146-170, 189-189, 198-205, 217-221, 273-273, 297-297, 341-341, 363-363, 412-430
crates/php_sys/rapira_classes.c (1)
5-85: LGTM!Also applies to: 106-179
crates/php_sys/rapira_dispatcher.c (1)
4-4: LGTM!Also applies to: 20-25, 72-82, 93-105, 173-204, 206-250, 252-271
crates/scoreboard/Cargo.toml (1)
3-4: LGTM!crates/php_sys/rapira_http.stub.php (3)
22-31: LGTM!Also applies to: 47-48, 67-75, 90-91
223-225: 🔒 Security & PrivacyReachability path
● Entry crates/runtime/src/lib.rs:164 to_request: Carried as raw bytes, like every other header value: php-src takes the │ ▼ ● Hop crates/php_sys/src/exchange.rs:473 state │ ▼ ● Hop crates/php_sys/src/handler.rs:48 handle: Bounded: spawn_blocking tasks cannot be cancelled and the │ ▼ ● Sink crates/php_sys/rapira_http.stub.phpNo change is required. The generated arginfo registers the
Dispatcher,DispatcherInfo, andExchangeconstructors as private.
110-127: 🗄️ Data Integrity & IntegrationNo change required. The generated arginfo contains both fully qualified address types, and C code assigns both
remoteandserver.crates/php_sys/src/exchange.rs (2)
98-146: LGTM!Also applies to: 192-233, 240-256, 261-318, 371-409, 411-430, 435-465, 467-527
334-366: 🔒 Security & PrivacyReachability path
● Entry crates/php_sys/src/rapira_worker.rs:120 rapira_worker: Can't run PHP. Answer one queued job with 503, then loop to │ ▼ ● Hop crates/php_sys/src/start.rs:71 start_worker: SAFETY: safe, trust me, I'm a developer │ ▼ ● Hop crates/php_sys/src/handler.rs:48 handle: Bounded: spawn_blocking tasks cannot be cancelled and the │ ▼ ● Sink crates/php_sys/src/exchange.rsThe C caller already screens Exchange response headers.
writeHeadrejects invalid names withwire_tokenand rejects CR, LF, and NUL in values withwire_valuebefore callingrapira_rs_exchange_write_head; no Rust-side change is required.crates/php_sys/src/handler.rs (1)
1-4: LGTM!Also applies to: 19-42, 101-114
crates/runtime/src/lib.rs (1)
194-194: LGTM!crates/php_sys/src/callbacks.rs (1)
17-20: LGTM!Also applies to: 73-82, 97-102, 117-121, 194-196, 230-233, 285-290, 322-322, 333-333, 345-345, 354-357, 400-401, 442-442, 460-461, 476-476
crates/php_sys/src/rapira_worker.rs (1)
68-78: LGTM!Also applies to: 154-156
crates/php_sys/src/start.rs (1)
2-7: LGTM!Also applies to: 17-28, 42-42, 70-70, 86-92, 100-112, 172-192, 194-205, 207-236, 238-257, 259-264
crates/php_sys/src/types.rs (2)
64-64: LGTM!Also applies to: 241-241
12-12: 🎯 Functional CorrectnessNo
Mode::Workerreferences remain.> Likely an incorrect or invalid review comment.crates/runtime/Cargo.toml (1)
3-4: 📐 Maintainability & Code QualityNo manifest change is required.
The root
Cargo.tomldefinesversionandeditionunder[workspace.package], so both inherited keys are valid.> Likely an incorrect or invalid review comment.
Reason for This PR
Description of Changes
__constructmethods. Regenerate stubs.License Acceptance
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).CHANGELOG.md.