Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better onWritable documentation, and more user friendly warning on ill-use. #915

Open
uNetworkingAB opened this issue May 29, 2023 · 2 comments

Comments

@uNetworkingAB
Copy link
Contributor

There is a mistake in the stricness of onWritable return checks. It really only checks if onWritable returns empty or non-empty, while the real check should be if it returns a boolean or or not. The return value of onWritable must be nothing but a boolean.

This check should also have a nice, clear error message like the others we have for the 2 other checks.

@uNetworkingAB
Copy link
Contributor Author

Ah so the problem is that an async function returns Promise, which is truthy. So this gets interpreted as true:

Writing nothing is always success, so by default you must return true.

So this is actually correct, as writing nothing should return true.

Only if you write inside onWritable, and fail, should you return false. So for async functions it works correctly already

@uNetworkingAB
Copy link
Contributor Author

Alright I remember now.

Returning false in onWritable is an optimization where the lib won't perform an extra attempt at sending, since false means the user just sent something themselves and failed. This means that all other returns than false, will try and drain the backpressure. This whole feature should be better documented to explain why onWritable wants boolean.

We should accept Promise, Boolean and only warn on empty return. Currently it errors with an ugly message on empty return.

@uNetworkingAB uNetworkingAB changed the title Add stricter onWritable return checks and errors Better onWritable documentation, and more user friendly warning on ill-use. May 29, 2023
@uNetworkingAB uNetworkingAB reopened this May 30, 2023
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

No branches or pull requests

1 participant