Details

src/main/scala/mesosphere/marathon/api/LeaderProxyFilter.scala
303	a response may have already been committed in this case. i'm pretty sure that these additional exception catches should be limited to the `copyRequestToConnection(...)` call. `copyConnectionResponse` already handles failures.

focusing error handling

jenkins requested changes to this revision.Aug 25 2017, 12:08 AM

This revision now requires changes to proceed.Aug 25 2017, 12:08 AM

✗ Build of 4118 failed jenkins-public-marathon-phabricator-869.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

Harbormaster failed to build B3375: Diff 4118!Aug 25 2017, 12:19 AM

jdef accepted this revision.Aug 25 2017, 12:22 AM

✗ Build of 4118 failed jenkins-public-marathon-phabricator-870.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

Harbormaster failed to build B3375: Diff 4118!Aug 25 2017, 12:55 AM

Thank you!

✗ Build of 4118 failed jenkins-public-marathon-phabricator-874.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

Harbormaster failed to build B3375: Diff 4118!Aug 25 2017, 5:54 PM

match requires a success case

jenkins requested changes to this revision.Aug 25 2017, 7:43 PM

This revision now requires changes to proceed.Aug 25 2017, 7:43 PM

ichernetsky accepted this revision.Aug 25 2017, 7:44 PM

jenkins accepted this revision.Aug 25 2017, 7:58 PM

This revision is now accepted and ready to land.Aug 25 2017, 7:58 PM

Harbormaster completed building B3380: Diff 4123.Aug 25 2017, 7:58 PM

This revision is now accepted and ready to land.Aug 25 2017, 7:58 PM

✔ Build of 4123 completed jenkins-public-marathon-phabricator-877.

You can create a DC/OS with your patched Marathon by creating a new pull
request with the following changes in buildinfo.json:

＼\ ٩( ᐛ )و /／

timcharper accepted this revision.Aug 25 2017, 8:50 PM

amen

Closed by commit rMARATHON6f5e6fc14378: community debug of proxy failure to leader. @xperimental code moved to master… (authored by kensipe). · Explain WhyAug 25 2017, 11:02 PM

This revision was automatically updated to reflect the committed changes.

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	4102	f6597c8		Aug 23 2017, 11:07 PM	★	★
Diff 2	4107	f6597c8	- adding logging as requested in review	Aug 24 2017, 4:47 PM	★	★
Diff 3	4108	2bd046f	- adding logging as requested in review	Aug 24 2017, 5:48 PM	★	★
Diff 4	4118	2bd046f	- focusing error handling	Aug 25 2017, 12:05 AM	★	★
Diff 5	4123	2bd046f	- match requires a success case	Aug 25 2017, 7:42 PM	★	★
Diff 6	4124	daa01d6	rMARATHON6f5e6fc14378d42f953ce34c2e446e17a86c1745	Aug 25 2017, 11:02 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
353815d927e0	c5c21f3cb834	63e9ae474c3b	Ken Sipe	match requires a success case	Aug 25 2017, 7:42 PM
63e9ae474c3b	9ea4d72febe9	fd583bd67b86	Ken Sipe	focusing error handling	Aug 25 2017, 12:05 AM
fd583bd67b86	6a60375b266b	420bc2982b88	Ken Sipe	adding logging as requested in review	Aug 24 2017, 4:47 PM
420bc2982b88	e73df69d34f8	2bd046f41b2a	Ken Sipe	community debug of proxy failure to leader. @xperimental code moved to master… (Show More…)	Aug 23 2017, 11:06 PM

1	1		package mesosphere.marathon
2	2		package api
3	3
4	4		import java.io.{ IOException, InputStream, OutputStream }
5	5		import java.net._
6	6		import javax.inject.Named
7	7		import javax.net.ssl._
8	8		import javax.servlet._
9	9		import javax.servlet.http.{ HttpServletRequest, HttpServletResponse }
10	10
11	11		import akka.Done
	12		import akka.http.scaladsl.model.StatusCodes._
12	13		import com.google.inject.Inject
13	14		import mesosphere.chaos.http.HttpConf
14	15		import mesosphere.marathon.core.election.ElectionService
15	16		import mesosphere.marathon.io.IO
16	17		import mesosphere.marathon.stream.Implicits._
17			import akka.http.scaladsl.model.StatusCodes._
18	18		import org.slf4j.LoggerFactory
19	19
20	20		import scala.annotation.tailrec
21			import scala.util.{ Failure, Success, Try }
22	21		import scala.util.control.NonFatal
	22		import scala.util.{ Failure, Success, Try }
23	23
24	24		/**
25	25		* Servlet filter that proxies requests to the leader if we are not the leader.
26	26		*/
27	27		class LeaderProxyFilter @Inject() (
28	28		httpConf: HttpConf,
29	29		electionService: ElectionService,
30	30		@Named(ModuleNames.HOST_PORT) myHostPort: String,
▲ Show 20 Lines • Show All 191 Lines • ▼ Show 20 Line(s)
222	222		IO.using(request.getInputStream) { requestInput =>
223	223		IO.using(leaderConnection.getOutputStream) { proxyOutputStream =>
224	224		copy(request.getInputStream, proxyOutputStream)
225	225		}
226	226		}
227	227		}
228	228		}
229	229
230			def copyRequestToConnection(leaderConnection: HttpURLConnection, request: HttpServletRequest): Unit = {
	230		def copyRequestToConnection(leaderConnection: HttpURLConnection, request: HttpServletRequest): Try[Done] = Try {
231	231		leaderConnection.setRequestMethod(request.getMethod)
232	232		copyRequestHeadersToConnection(leaderConnection, request)
233	233		copyRequestBodyToConnection(leaderConnection, request)
	234		Done
234	235		}
235	236
236	237		def cloneResponseStatusAndHeader(remote: HttpURLConnection, response: HttpServletResponse): Try[Done] = Try {
237	238		val status = remote.getResponseCode
238	239		response.setStatus(status)
239	240
240	241		Option(remote.getHeaderFields).foreach { fields =>
241	242		// headers and values can both be null :(
Show All 28 Lines
270	271
271	272		try {
272	273		if (hasProxyLoop) {
273	274		log.error("Prevent proxy cycle, rejecting request")
274	275		response.sendError(BadGateway.intValue, ERROR_STATUS_LOOP)
275	276		} else {
276	277		val leaderConnection: HttpURLConnection = createAndConfigureConnection(url)
277	278		try {
278			copyRequestToConnection(leaderConnection, request)
279			copyConnectionResponse(
280			response
281			)(
	279		copyRequestToConnection(leaderConnection, request) match {
	280		case Failure(ex: ConnectException) =>
	281		log.error(ERROR_STATUS_CONNECTION_REFUSED, ex)
	282		response.sendError(BadGateway.intValue, ERROR_STATUS_CONNECTION_REFUSED)
	283		return
	284		case Failure(ex: SocketTimeoutException) =>
	285		log.error(ERROR_STATUS_GATEWAY_TIMEOUT, ex)
	286		response.sendError(GatewayTimeout.intValue, ERROR_STATUS_GATEWAY_TIMEOUT)
	287		return
	288		case Failure(ex) =>
	289		log.error(ERROR_STATUS_BAD_CONNECTION, ex)
	290		response.sendError(InternalServerError.intValue)
	291		return
	292		case Success(_) => // ignore
	293		}
	294		copyConnectionResponse(response)(
282	295		() => cloneResponseStatusAndHeader(leaderConnection, response),
283	296		() => cloneResponseEntity(leaderConnection, response)
284	297		)
285			} catch {
286			case connException: ConnectException =>
287			response.sendError(BadGateway.intValue, ERROR_STATUS_CONNECTION_REFUSED)
288	298		} finally {
		jeschkiesUnsubmitted Done Just wondering, shouldn't we log the errors?
		kensipeAuthorUnsubmitted Not Done you are right! thx!
289	299		Try(leaderConnection.getInputStream.close())
290	300		Try(leaderConnection.getErrorStream.close())
291	301		}
292	302		}
293	303		} finally {
		jdefUnsubmitted Not Done a response may have already been committed in this case. i'm pretty sure that these additional exception catches should be limited to the `copyRequestToConnection(...)` call. `copyConnectionResponse` already handles failures.
294	304		Try(request.getInputStream.close())
295	305		Try(response.getOutputStream.close())
296	306		}
297	307
298	308		}
299	309
300	310		def copy(nullableIn: InputStream, nullableOut: OutputStream): Unit = {
301	311		try {
Show All 10 Lines
312	322
313	323		object JavaUrlConnectionRequestForwarder {
314	324		private val log = LoggerFactory.getLogger(getClass)
315	325
316	326		/** Header for proxy loop detection. Simply "Via" is ignored by the URL connection.*/
317	327		val HEADER_VIA: String = "X-Marathon-Via"
318	328		val ERROR_STATUS_LOOP: String = "Detected proxying loop."
319	329		val ERROR_STATUS_CONNECTION_REFUSED: String = "Connection to leader refused."
	330		val ERROR_STATUS_GATEWAY_TIMEOUT: String = "Connection to leader timed out."
320	331		val ERROR_STATUS_BAD_CONNECTION: String = "Failed to successfully establish a connection to the leader."
321	332
322	333		val HEADER_FORWARDED_FOR: String = "X-Forwarded-For"
323	334		final val NAMED_LEADER_PROXY_SSL_CONTEXT = "JavaUrlConnectionRequestForwarder.SSLContext"
324	335
325	336		def copyConnectionResponse(response: HttpServletResponse)(
326	337		forwardHeaders: () => Try[Done], forwardEntity: () => Unit): Unit = {
327	338
Show All 11 Lines

community debug of proxy failure to leader. @xperimental code moved to master first.
ClosedAll Users
Actions

Details

Diff Detail

＼\ ٩( ᐛ )و /／

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

＼\ ٩( ᐛ )و /／

Revision Contents

Diff 4124

src/main/scala/mesosphere/marathon/api/LeaderProxyFilter.scala

community debug of proxy failure to leader. @xperimental code moved to master first.ClosedAll UsersActions

Details

Diff Detail

＼\ ٩( ᐛ )و /／

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

(๑′°︿°๑)

＼\ ٩( ᐛ )و /／

Revision Contents

Diff 4124

src/main/scala/mesosphere/marathon/api/LeaderProxyFilter.scala

community debug of proxy failure to leader. @xperimental code moved to master first.
ClosedAll Users
Actions