Add Plug.Conn.register_before_chunk/2

[feynmanliang]

Resolves #1153

An example use case where we cache the response body for a regular HTTP request and individual chunks for a chunked response like SSEs:

              # register lifecycle callback to cache response
              conn
              |> register_before_send(fn conn ->
                if conn.resp_body do
                  Dreamcatcher.SemanticCache.API.put(key, conn.resp_body)
                end

                conn
              end)
              |> register_before_chunk(fn conn, chunk ->
                if chunk do
                  Dreamcatcher.SemanticCache.API.append_chunk(key, chunk)
                end

                conn
              end)

[whatyouhide]

I generally like the idea and can't think of a good reason not to do this.

[whatyouhide]

Ah, we also need to add tests for:

• the AlreadySentError exception
• the error that you raised when the state of the conn changes
• a test where you modify the connection in one register_before_chunk/2 callback (like adding a header or something) and you make sure that the new connection is passed to further callbacks

[feynmanliang]

I added a test for 1 (https://github.com/elixir-plug/plug/pull/1154/files#diff-5115eb93e759386d8da8b04d13db4e24d30f2d9ece59b85fe63068236aac9dfeR1452-R1458). For 2-3, almost all the methods which modify Conn raise an AlreadySentError since conn.state == :chunked fails the state in @unsent guard.

We could:

1. Modify the conn manually inside the callback for the purpose of the test, but that feels a bit contrived and not representative of real world use.
2. Or alternatively since the initial response has been sent having a conn argument could be misleading misleading and we could remove it alongside the extra logic (ArugmentError, merging headers) in run_before_chunk.
3. Or... we could add :chunked as an @unsent state. But I don't think this is semantically correct, since the connection status / headers / cookies will have already been sent by the earlier call to send_chunked.

I prefer 2, but curious if you had any thoughts or if I am missing something?

[josevalim]

I think we should simply document the conn is read-only, as you can't really do anything useful with them. The main goal is to get a copy of any assigns or headers stored in it.

[josevalim]

Actually, scratch that, I can see a connection is being returned all the way up.

@feynmanliang you can set a assigns/private field, like a counter, and measure it is being bumped. That should be supported :)

[feynmanliang]

Sounds good, I've exercised that in https://github.com/elixir-plug/plug/pull/1154/files#diff-5115eb93e759386d8da8b04d13db4e24d30f2d9ece59b85fe63068236aac9dfeR427-R444

[josevalim]

💚 💙 💜 💛 ❤️