Skip to content

Commit

Permalink
finagle-http2: Handle upgrade events properly in APCH
Browse files Browse the repository at this point in the history
Problem / Solution

The AdapterProxyChannelHandler is relying on a Netty but that was fixed
sometime between version 4.1.35 and 4.1.37. We're not upgrading to 4.1.37
but we should fix the behavior anyway.

JIRA Issues: CSL-8436

Differential Revision: https://phabricator.twitter.biz/D336428
  • Loading branch information
Bryce Anderson authored and jenkins committed Jul 23, 2019
1 parent 0ef03b8 commit b06fab3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ Breaking API Changes
of `GenSerialClientDispatcher` and `SerialClientDispatcher` have been removed.
``PHAB_ID=D342883``

Runtime Behavior Changes
~~~~~~~~~~~~~~~~~~~~~~~~

* finagle-core: Failed reads on Linux due to a remote peer disconnecting should now be properly
seen as `c.t.f.ChannelClosedException` instead of a `c.t.f.UnknownChannelException`.
``PHAB_ID=D336428``

19.7.0
------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ private[finagle] object IOExceptionStrings {
"Broken pipe", // Found on linux
"An existing connection was forcibly closed by the remote host", // Found on windows
"syscall:read(..) failed: Connection reset by peer", // Found on linux w/ native epoll
"readAddress(..) failed: Connection reset by peer", // Found on linux w/ native epoll
"writeAddress(..) failed: Connection reset by peer" // Found on linux w/ native epoll
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package com.twitter.finagle.http2.transport

import com.twitter.finagle.FailureFlags
import com.twitter.finagle.http2.transport.StreamMessage._
import com.twitter.finagle.stats.{StatsReceiver, NullStatsReceiver}
import com.twitter.logging.{Logger, Level, HasLogLevel}
import com.twitter.finagle.stats.{NullStatsReceiver, StatsReceiver}
import com.twitter.logging.{HasLogLevel, Level, Logger}
import io.netty.channel._
import io.netty.channel.embedded.EmbeddedChannel
import io.netty.handler.codec.http.HttpClientUpgradeHandler.UpgradeEvent
import io.netty.handler.codec.http.{HttpObject, LastHttpContent}
import io.netty.util.ReferenceCountUtil
import io.netty.util.concurrent.PromiseCombiner
Expand Down Expand Up @@ -153,6 +154,7 @@ private[http2] final class AdapterProxyChannelHandler(
case Ping => ctx.fireChannelRead(Ping)
case exn: StreamException => ctx.fireChannelRead(exn)
case upgrade: Http2UpgradingTransport.UpgradeResult => ctx.fireChannelRead(upgrade)
case upgrade: UpgradeEvent => ctx.fireChannelRead(upgrade)
case _ =>
val wrongType = new IllegalArgumentException(
s"Expected a StreamMessage or UpgradeEvent, got ${msg.getClass.getName} instead."
Expand Down

0 comments on commit b06fab3

Please sign in to comment.