expose processClient

[disruptek]

This allows a server to be written that can capture exceptions yielded by this procedure without leaving its event loop.

[narimiran]

This is a change in API, not a (critical) bug fix — I don't think it should have [backport] tag, and the change/addition should be mentioned in the changelog.

(For future PRs: even it were just a simple bugfix, CIs should definitely run on it, so no [no ci] either)

[dom96]

I don't like this change. Can you elaborate on what you're trying to achieve and why you need to this to be exported?

capture exceptions yielded by this procedure without leaving its event loop.

I don't really follow this reasoning at all.

[dom96]

So I believe I ran into something very similar to what you're trying to fix. The use case is as follows:

• I'm running the async event loop in my program
• I want to run a temporary HTTP server
• If I close that temporary HTTP server it'll crash my main async event loop

To me, exporting processClient and recreating the logic in the serve proc is just a workaround for this. The crux of the problem is that we all use asyncCheck, which will crash our event loop. Why don't we introduce an asyncExceptionHandler procedure that can be overridden and have asyncCheck call it whenever there is a problem? This way we can ensure we can handle this problem across all libraries.

[disruptek]

Yes, it's a workaround by design. The problem you ran into seems more severe, unless I've missed something.

The problem this is trying to fix is with a more trivial "user level" async scenario that is actually the result of the design choices in the server loop itself. It's easily avoidable if one were to rewrite the loop, which is what the PR enables.

FWIW, you don't need to convince me: the export of processClient is intended to serve two people's needs:

• zedeus needs a server that doesn't crash when an async client handler crashes
• dom96 needs interface changes that break minimal existing code

What does this mythical handler do to callers of asyncCheck in the event of an exception? Does that meet the needs of zedeus and dom96?

[dom96]

We could use zedeus' opinion in here :)

[disruptek]

@zedeus ping Zed

[zedeus]

My use case was simply using Jester with asynchttpserver, nothing more than that, not involving asyncCheck. I gave up on it after every request caused a crash when running behind nginx, I should probably investigate that and open an issue.

[disruptek]

For posterity, here is the reproduction (which does not require nginx): https://github.com/zedeus/jester-example

[dom96]

zedeus' issue seems like a bug in asynchttpserver. Working around it by using this new API and handling the Bad FD errors is not the right solution.

[dom96]

@disruptek @zedeus Do we have an issue which tracks this anywhere? here or Jester's repo?

[zedeus]

I haven't had time to investigate the problem further so I haven't opened one, but I can do it now with the limited info I have