Update SignalR transport protocol spec

[mikaelm12]

Blocked on: #13773

[davidfowl]

We're not merging this change until we merge the feature right?

[mikaelm12]

We're not merging this change until we merge the feature right?

Yup

[analogrelay]

There are references to "Connection ID" lower down that should be updated to refer to Connection Token.

Also, the spec should describe what a server is required to do when an old client arrives. Which is, it must either:

1. Respond with a single connectionId that is usable as both token and ID, or
2. Respond with an error payload indicating the older version is not supported by this server.

[analogrelay]

I'd like to briefly discuss how we are going to handle versioning this spec. Should we just make this the version 2 spec and use git history? Or should we have sections where we call out version differences in a single "living" spec that covers all versions? I lean towards the latter.

[BrennanConroy]

Or should we have sections where we call out version differences in a single "living" spec that covers all versions?

I prefer this one. Do we place version headers on each sub-section that has differences and explain the behavior for each version under that header?

Example:

Some section

Version 1.0

Things work

Version 2.0

Things no longer work

[analogrelay]

This name is odd to me. It's more like a protocolVersion since it's the version of the protocol, not the version of "negotiation".

[halter73]

I think most of us preferred protocolVersion, but shied away from from using that since we thought what what the old client used and we wanted to be able to easily distinguish clients from request logs.

It turns out the old client used clientProtocol as the QS parameter, so I'm also in support of protocolVersion.

[BrennanConroy]

The ASP.NET SignalR negotiate response includes protocolVersion

https://github.com/SignalR/SignalR/blob/fc475fa161eaf94150c4aaa4ba71830fd976eb6c/src/Microsoft.AspNet.SignalR.Client/NegotiationResponse.cs#L14

[analogrelay]

Version 0 won't have a version in the response at all, right?

[BrennanConroy]

Client could receive this if talking to a new server

[analogrelay]

It feels like the negotiateVersion (or whatever it's called) should just not be present in that case. Our clients don't bork if extra properties are present but others might. We can't guarantee that other clients will be compatible, but we can work to avoid breaking them when we can.

Would also be good to put an explicit thing in about clients being required to accept extra JSON properties.

[halter73]

I think as a general rule, we should require client to ignore unknown properties in payloads and what order the properties arrive in.

While it's possible for the server to use the requested and/or selected negotiateVersion to generate exactly the payload the client expects, I'd rather not go through that extra effort. There also may come a time when we want to add a property to a payload without incrementing and special-casing yet another the protocol version.

[BrennanConroy]

Lets decide on this quickly, now that the PR is out for 3.1

[halter73]

You know my position, but I certainly won't push back hard if people feel we should omit the negotiateVersion in this case.

[analogrelay]

I think as a general rule, we should require client to ignore unknown properties in payloads and what order the properties arrive in.

I do entirely agree with this. It's just a new requirement we're putting on external clients that they may not have been prepared for. Leaving the "version 0" responses untouched protects us from that problem while allowing "version 1 and up" to add the requirement that clients ignore extra fields.

Buuuutt.... I don't care a lot. It should be safe to leave this and patch it out again if a client does come along that requires us to strip this property.

Leave it in. Let's land this ✈.