Conversation
|
Looks like |
45cbd50 to
fec70c5
Compare
There was a problem hiding this comment.
Pull request overview
This PR automatically enables ignore_user_abort for worker threads, eliminating the need for users to manually configure this setting via INI or in their worker scripts.
Changes:
- Automatically sets
ignore_user_abortto true for worker threads and restores the original setting for non-worker threads - Adds a test to verify workers continue execution after connection abort
- Removes documentation instructing users to manually call
ignore_user_abort(true)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frankenphp.c | Stores original ignore_user_abort setting and automatically enables it for worker threads |
| worker_test.go | Adds test case verifying worker continues after client connection abort |
| docs/worker.md | Removes manual ignore_user_abort(true) call from documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert.Equal(t, body1, "requests:1", "should have handled exactly one request") | ||
| body2, _ := testGet("http://example.com/worker-with-counter.php", handler, t) | ||
|
|
||
| assert.Equal(t, body2, "requests:2", "should not have stopped execution after the first request was aborted") |
There was a problem hiding this comment.
The assert.Equal arguments are in the wrong order. The convention is assert.Equal(t, expected, actual, message). Should be assert.Equal(t, 'requests:1', body1, message).
| assert.Equal(t, body1, "requests:1", "should have handled exactly one request") | |
| body2, _ := testGet("http://example.com/worker-with-counter.php", handler, t) | |
| assert.Equal(t, body2, "requests:2", "should not have stopped execution after the first request was aborted") | |
| assert.Equal(t, "requests:1", body1, "should have handled exactly one request") | |
| body2, _ := testGet("http://example.com/worker-with-counter.php", handler, t) | |
| assert.Equal(t, "requests:2", body2, "should not have stopped execution after the first request was aborted") |
| assert.Equal(t, body1, "requests:1", "should have handled exactly one request") | ||
| body2, _ := testGet("http://example.com/worker-with-counter.php", handler, t) | ||
|
|
||
| assert.Equal(t, body2, "requests:2", "should not have stopped execution after the first request was aborted") |
There was a problem hiding this comment.
The assert.Equal arguments are in the wrong order. The convention is assert.Equal(t, expected, actual, message). Should be assert.Equal(t, 'requests:2', body2, message).
| assert.Equal(t, body1, "requests:1", "should have handled exactly one request") | |
| body2, _ := testGet("http://example.com/worker-with-counter.php", handler, t) | |
| assert.Equal(t, body2, "requests:2", "should not have stopped execution after the first request was aborted") | |
| assert.Equal(t, "requests:1", body1, "should have handled exactly one request") | |
| body2, _ := testGet("http://example.com/worker-with-counter.php", handler, t) | |
| assert.Equal(t, "requests:2", body2, "should not have stopped execution after the first request was aborted") |
fec70c5 to
3a17d41
Compare
Automatically sets
ignore_user_abortto true in worker mode as mentioned in #2186, removing the requirement to change it via ini.Would also be possible to expose something like an explicit
frankenphp_client_has_closed()function for in-between critical sections.