Skip to content

feat: always ignore user abort#2189

Open
AlliBalliBaba wants to merge 2 commits intomainfrom
fix/always-ignore-user-abort
Open

feat: always ignore user abort#2189
AlliBalliBaba wants to merge 2 commits intomainfrom
fix/always-ignore-user-abort

Conversation

@AlliBalliBaba
Copy link
Contributor

Automatically sets ignore_user_abort to 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.

@AlliBalliBaba
Copy link
Contributor Author

Looks like containerimage.config.digest is sometimes empty, not sure what has changed, maybe something in upstream images.

@dunglas dunglas force-pushed the fix/always-ignore-user-abort branch from 45cbd50 to fec70c5 Compare February 16, 2026 13:52
@dunglas dunglas requested a review from Copilot February 16, 2026 13:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_abort to 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.

Comment on lines +171 to +174
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")
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
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")

Copilot uses AI. Check for mistakes.
Comment on lines +171 to +174
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")
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
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")

Copilot uses AI. Check for mistakes.
@dunglas dunglas force-pushed the fix/always-ignore-user-abort branch from fec70c5 to 3a17d41 Compare February 16, 2026 17:24
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.

4 participants