Simplify API

[ronag]

I see undici primarily as a low level library with some common sense APIs to build more advanced things on top of. Hence I believe our priorities are (in order of importance) correctness, performance and maintainability.

The current API has a lot of convenience methods and properties that bring maintenance burden. I would like to challenge us to try and remove some to reduce duplication of functionality and improve maintainability.

I propose to remove the following properties and methods:

• Dispatcher.close

• Pool.close()

• Pool.closed

• Pool.url

• Pool.connected

• Pool.pending

• Pool.size

• Pool.running

• Pool.busy

• Client.close()

• Client.closed

• Client.url

• Client.connected

• Client.pending

• Client.size

• Client.running

• Client.busy

• Agent.close

• Agent.connected

• Agent.closed

• Agent.pending

• Agent.size

• Agent.running

Reducing the API surface makes:

• Documentation simpler and easier to understand.
• Easier to implement custom Dispatcher.
• Easier to maintain and less bugs in undici.

In order to not lose functionality and make the above still achievable in user land I propose to add the following events:

• 'pending' - on request queued
• 'running' - on request running
• 'complete'- on request complete
• 'failed' - on request failed

These events are something I would like to add regardless of the above for better introspection.

Thoughts? Please do push back. I know I have a bias towards simplicity at the cost of functionality and convenience.

[ronag]

@szmarczak @dnlup @Ethan-Arrowood @mcollina

[szmarczak]

I like the idea of simplicity but isn't that too simple? For example .connected and .closed seem pretty useful to me. .url, .busy as well. What I don't like is the names of .pending and .running - they are synonyms.

I believe our priorities are (in order of importance) correctness, performance and maintainability.

In terms of performance, how better is it? And about maintainability - would that be a huge refactor?

[Ethan-Arrowood]

i do appreciate a simpler API. As we discussed in the past I find many of the existing properties misleading and confusing (for example, connected is not a boolean state but a property referring to how many things are connected to the pool.

[mcollina]

Most of those properties are needed in one way or another to implement the level above in a performant or reliable way.

All of them are useful for me. A few are more important than others, e.g. close() and closed are fundamental for my main use case of Undici.

It's not necessary for them to be part of the Dispatcher API in any way.

[ronag]

Let me rephrase then. Is there any you would consider unnecessary?

[mcollina]

I think busy, pending, running, size and connected are deeply confusing. As the key functionality of "busy" is implemented the 'drain' event, most of those are not needed. I think we should provide some way to have a stats property with all those numbers aggregated. This would be useful for logging/promethetus/etc...

[ronag]

Should we remove those properties then?

I'm not sure about stats but I guess that's something we could add in a semver minor? Ditto with the events i proposed. wdyt?

[mcollina]

+1. We should open an issue called "runtime performance metrics" and discuss them there.