Reorder context, future-proof for a tracing context world

[ktoso]

In light of https://github.com/swift-server/async-http-client/pull/227/files#r429299831 and a number of long discussions and pondering about this...

I think we'll get the best APIs and outcome for all server libs if we stick to one style, and the "context last" indeed has some gains in Swift... I can go in depth about what convinced me today after hours of chatting with Johannes and looking at my own APIs.

This proposes to reshape the API to become:

Lambda.run { (payload: String, context, callback) in 
// this perhaps makes most sense as consistent with "last, 
// but before trailing closures (regardless who calls them)"?

// OR?
Lambda.run { (payload: String, callback, context) in

and we'd encourage the same shape in APIs I work on myself, AsyncHTTPClient, and the Baggage / Context work that's ongoing with GSoC/Moritz. (Swift gRPC already fits).

(Pretty sure people have strong feelings about this so we can talk it over?)

Specific rule wise I think we'd end up with:

• "last"
• but before trailing closures

Believe me, it took a lot of convincing for me to change my mind... As long as we end up on the same style there will be great benefits for consistency of looks of server swift code though.

[ktoso]

Holding for now -- seems we should gather a wider consensus and then commit and go all in with all libs we control.

[weissi]

I'm just copy & pasting my arguments from the AsyncHTTPClient PR comments:

---

Swift has many APIs that read like addObserver<O: Observer>(_ observer: O). If we added context as the first parameter, that becomes a huge mess:

• addObserver<O: Observer>(context: Context, _ observer: O) is just wrong
• addObserver<O: Observer>(context: Context, observer: O) is less bad but much longer than necessary
• addObserver<O: Observer>(_ observer: O, context: Context) has exactly the right feel in Swift

More importantly, you're only supposed to add the "Observer" bit when you're "compensating for weak type information". So in many APIs where it's much clearer, you're supposed to just name the method add.

Example: add(_ rectangle: Rectangle). If we now add context, we end up in a very bad spot:

• add(context: Context, _ rectangle: Rectangle) is again just wrong because we're adding a rectangle and not a context
• add(context: Context, rectangle: Rectangle) is less bad but super confusing
• add(_ rectangle: Rectangle, context: Context) is again as good as it gets.

Basically, you cannot just skip over context when it's at the front but it works very well if at the back.

[Andrea-Scuderi]

I think here we should follow the convention of other Lambda frameworks.

(event, context)
https://docs.aws.amazon.com/lambda/latest/dg/nodejs-handler.html
https://docs.aws.amazon.com/lambda/latest/dg/python-context.html

In addition, I also suggest to change payload with event.

In my opinion we need to simplify the learning curve by adopting common usage pattern used in other languages.

[weissi]

@ktoso do you agree we should do last now? Or need more data?

[glbrntt]

Some more data: in gRPC Swift we follow the context-last-except-for-trailing-closures rule:

In server RPC handlers where users implement the service logic:

• unary, server streaming: context is last
• client streaming, bidirectional streaming: context is the only argument

These contexts vary by the RPC type but broadly provide things like the event loop and ways to send messages as well as a logger.

For client RPCs:

• unary: context is last
• client streaming: context is the only argument
• server streaming, bidirectional streaming: context is last except for trailing closure

These are all “CallOptions” (i.e. timeout/deadline, encoding, metadata), more or less a context.

[ktoso]

Yes I've collected info from various libs (including gRPC) and run it by Kyle (stdlib) and seems we're converging on that. Still need to sanity check with some folks before it's "the official stance of the swift team" or whatnot, but that'll happen after wwdc.

The specific rule we arrived at is a bit more detailed, as takes into account defaulted parameters etc, and it seems to be:

Argument naming/positioning

In order to propagate baggage through functions it may (very often) be necessary to pass it explicitly. We provide the following style guidance about what the “most Swifty” way of dealing with context is:

Assuming the general parameter ordering of Swift function is as follows (except DSL exceptions):

• Required non-function parameters
• Defaulted non-function parameters
• Required function parameters
• Defaulted function parameters

Context should be: the last parameter in the required non-function parameters group in a function declaration.

This way when reading the call side, users can simply ignore the context and the method remains “Swifty”.

(see slashmo/gsoc-swift-tracing#12 )