Remove blank line after subscription headers

canadaduane commented 3 years ago

When a client makes a GET request with Subscribe: header set, the server responds without version, parents, and merge-type headers in the canonical response headers. Instead, they are sent in a subsequent block that is only accessible with special braid-header ("virtual header") parsing.

https://github.com/braid-work/braid-spec/blob/bf179d8c84ea48b5cb1ea4faf23c6ea9bfc28416/draft-toomim-httpbis-braid-http-03.txt#L406-L472

What if we combine the first version's headers with the canonical HTTP response headers? See PR #84 for concrete change.

This would have the following desirable properties:

A non-Braid-aware fetch could still fetch a Braid resource and parse Version, Parents, and Merge-Type headers.
A regular GET and a Subscribe-GET could have the same headers and (initial) body. This simplifies implementation details (e.g. the "if subscribe do braid http / else do regular http" block could become "do braid http; continue sending versions if subscribe").

toomim commented 3 years ago

I think this is a very interesting proposal! Good idea, Duane.

I especially appreciate point (2). I am not yet sure that point (1) will work, because we are returning a different status code and aren't closing the connection after sending the body, but that's an empirical question that I'd love to learn the answer to.

I want to think on this for a while and then give my thoughts, but I'm quite intrigued.

toomim commented 3 years ago

I just realized that this conflicts with the specification for the "all-caught-up" version: https://github.com/braid-work/braid-spec/pull/79/files

         HTTP/1.1 209 Subscription
          Subscribe: keep-alive
          Version: "ej4lhb9z78"                       <-- Current Version

          Version: "ej4lhb9z78"                       + Stream of updates
          Parents: "oakwn5b8qh", "uc9zwhw7mf"         |
          Content-Type: application/json              |
          Merge-Type: sync9                           |
          Content-Length: 64                          |
                                                      |
          [{"text": "Hi, everyone!",                  |
            "author": {"link": "/user/tommy"}}]       V
                                                      <-- Now caught up

That first Version: "ej4lhb9z78" needs to be separated by a blank line in order to be meaningful here. If we remove the blank line, we should probably rename the first Version to Initial-Version or something.

This also brings up another issue to consider— it's common for a server to send not the current version to a new connecting client, but rather a whole set of versions. This is important, for instance, when other clients might be actively editing the resource, and might create a patch based on an old version that the new connecting client needs to know about.

The unfortunate consequence of this is that it becomes less conceptually elegant to group the "first version" in with the initial response. If a client that doesn't know about Braid Subscriptions connects, and just looks at the initial version that comes across the wire, it'll actually be looking at an old out-of-date version. The current version will often come later in the stream.

canadaduane commented 3 years ago

That first Version: "ej4lhb9z78" needs to be separated by a blank line in order to be meaningful here. If we remove the blank line, we should probably rename the first Version to Initial-Version or something.

Does the addition of the Current-Versions header resolve the above concern?

https://github.com/braid-work/braid-spec/commit/182e8a6488d565a2baec3d9bf2870c5aeca71748

toomim commented 3 years ago

Yes, it does resolve it for me! Although I do wonder if Current-Versions is a name that we want to stick with? I don't recall us discussing that name fully.

canadaduane commented 3 years ago

I like Current-Versions but if we need to discuss maybe we can open another issue.

toomim commented 3 years ago

Notes from https://braid.org/meeting-3:

Cases that interact with this proposal:

First message is document snapshot
Client receives all operations
- In the first message
- In the first N messages
Client reconnects and wants changes since last time

Question:

Is it possible for non-braid HTTP clients to fall back to interpreting subscription requests as a single version?
- The subscription response 209 might break this

Use-case:

Could you have a poor-man's subscribe by polling?
- Mike: I can think of how to do this, and would like to write up an explanation. This doesn't need the subscribe header.
- Seph: We could also do a hybrid approach by saying "Subscribe, but limit me to N messages/bytes", and then the client can poll opening a new connection for a new amount.

josephg commented 3 years ago

Punt to 04

toomim commented 11 months ago

(Removing Chair hat:) After some thought, I think this is a very good improvement. I'm on board!

This proposal will clarify that there are only TWO levels of data in Braid-HTTP subscriptions:

Updates
Patches

This will make things much more elegant. Notice that this will make GET and GET Subscribe responses look identical through the first update, aside from the Subscribe: header and status code.

Normal GET:

      Request:

         GET /hello

      Response:

         HTTP/1.1 200 OK
         Content-Type: application/json                     | Update
         Content-Length: 12                                 |
                                                            |
         Hello world!                                       | | Snapshot

GET + Subscribe:

      Request:

         GET /hello
         Subscribe: true

      Response:

         HTTP/1.1 209 Subscription
         Subscribe: true
         Content-Length: 12                                 | Update
                                                            |
         Hello world!                                       | | Snapshot

         Content-Length: 14                                 | Update
                                                            |
         Goodbye world!                                     | | Snapshot

I think the symmetry we're creating there is a very good thing.

The blank line in Subscriptions has been giving the impression that there are three levels of headers (e.g. see https://github.com/braid-org/braid-spec/issues/96#issue-815138063):

Top-level resource headers
Update-level headers, for each new version
Patch-level headers

However, I claim that there's no need for a distinction between 2 and 3. This false distinction has been confusing people. I think we should remove it.

Where this blank line came from

I think the blank line was probably introduced into our code accidentally while implementing, because when we use existing HTTP libraries, we have to generate and parse headers within the "body" of a message from the library's perspective, and the library automatically inserts a blank line between its headers and body. When we generated version: headers inside the body, we neglected to notice that the library inserted a blank line in between those headers and its own headers.

We'll need to modify our implementations so that they generate the headers for the first update into the existing HTTP header libraries, and then generate subsequent headers into the body of the message. And when reading messages, our parsers will need to copy the initial update's headers from the library's headers list, and then parse the headers for subsequent updates itself.

toomim commented 11 months ago

Concluding the other issues brought up in Meeting-3

These issues are orthogonal:

Cases that interact with this proposal:

First message is document snapshot

^ This is already an allowed server behavior, but we'll want a way for clients to request it when initiating a Subscription.

Client receives all operations

In the first message

In the first N messages

^ Denoting the initial set of operations is addressed by the "all-caught-up" Current-Version: header. https://github.com/braid-org/braid-spec/pull/79

Client reconnects and wants changes since last time

^ This is handled with the Parents: header.

The following are suggestions for parameters that we'll want to add to the Subscribe header.

Question:

Is it possible for non-braid HTTP clients to fall back to interpreting subscription requests as a single version?

The subscription response 209 might break this Use-case:

Could you have a poor-man's subscribe by polling?

Mike: I can think of how to do this, and would like to write up an explanation. This doesn't need the subscribe header.

Seph: We could also do a hybrid approach by saying "Subscribe, but limit me to N messages/bytes", and then the client can poll opening a new connection for a new amount.

toomim commented 11 months ago

I'm re-nominating this for consideration in draft 03, because:

the change to the spec should be very simple to author (it's just removing a blank line)
it makes the protocol much simpler to read: it's clearly 2 levels of updates, not 3

toomim commented 11 months ago

Notes from Meeting-67:

Rahul: the blank line could provide semantics of "this is the initial state" vs. "this is an update"

toomim commented 11 months ago

I've drafted this up in PR https://github.com/braid-org/braid-spec/pull/121

CxRes commented 11 months ago

To expand on my suggestion:

A blank line after headers can be used to indicate to the client that there is no initial representation in this response and only updates will be sent.
You can think of the blank line as placeholder/proxy for the initial representation.
If an initial representation is sent, there are no blank lines. This will be consistent with normal GET.

toomim commented 11 months ago

there is no initial representation in this response and only updates will be sent

@CxRes are you describing the situation described in https://github.com/braid-org/braid-spec/issues/105? What is a scenario in which there would be no initial representation? The one that comes to mind for me is that you subscribe to a resource with a Parents: header specifying the current version; thus there would be no new updates to send until a new change occurs. Is that what you are thinking of?

CxRes commented 11 months ago

Something like that. Suppose a client has already obtained a cached copy. They could subscribe with a request where they explicitly ask for a representation to be sent only if the event-id/version has changed, otherwise only updates be sent.

I have explicitly covered this in PREP. See implementation guidance under section 7 for such a Request and section 9.2 for a corresponding response.

toomim commented 10 months ago

I implemented this, and found it breaks with chrome'sfetch(). Details here: https://github.com/braid-org/braid-spec/pull/121#issuecomment-1722370972

braid-org / braid-spec

Remove blank line after subscription headers #85

Where this blank line came from

Concluding the other issues brought up in Meeting-3