From e219d6a94f6accbc907a3b3cd0537d1e6e9226fb Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Wed, 21 Jan 2015 22:55:00 +0900
Subject: [PATCH] nghttpx: Clarify error_reply upstream method

There is no application level failure in h1 upstream.  For h2, SPDY
upstreams, don't call DIE(), instead return -1 to delete handler.
---
 src/shrpx_http2_upstream.cc |  2 +-
 src/shrpx_https_upstream.cc | 24 ++++++------------------
 src/shrpx_https_upstream.h  |  2 +-
 src/shrpx_spdy_upstream.cc  |  2 +-
 4 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc
index ac1d5b61..8c3ae005 100644
--- a/src/shrpx_http2_upstream.cc
+++ b/src/shrpx_http2_upstream.cc
@@ -1087,7 +1087,7 @@ int Http2Upstream::error_reply(Downstream *downstream,
   if (rv < NGHTTP2_ERR_FATAL) {
     ULOG(FATAL, this) << "nghttp2_submit_response() failed: "
                       << nghttp2_strerror(rv);
-    DIE();
+    return -1;
   }
 
   return 0;
diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc
index 3da789b3..92a3d2b2 100644
--- a/src/shrpx_https_upstream.cc
+++ b/src/shrpx_https_upstream.cc
@@ -309,9 +309,7 @@ int HttpsUpstream::on_read() {
         handler->set_should_close_after_write(true);
         // Following paues_read is needed to avoid reading next data.
         pause_read(SHRPX_MSG_BLOCK);
-        if (error_reply(503) != 0) {
-          return -1;
-        }
+        error_reply(503);
         handler_->signal_write();
         // Downstream gets deleted after response body is read.
         return 0;
@@ -363,9 +361,7 @@ int HttpsUpstream::on_read() {
         status_code = 400;
       }
 
-      if (error_reply(status_code) != 0) {
-        return -1;
-      }
+      error_reply(status_code);
 
       handler_->signal_write();
 
@@ -470,9 +466,7 @@ int HttpsUpstream::downstream_read(DownstreamConnection *dconn) {
 
   if (downstream->get_response_state() == Downstream::MSG_BAD_HEADER) {
     handler_->set_should_close_after_write(true);
-    if (error_reply(502) != 0) {
-      return -1;
-    }
+    error_reply(502);
     downstream->pop_downstream_connection();
     goto end;
   }
@@ -539,9 +533,7 @@ int HttpsUpstream::downstream_eof(DownstreamConnection *dconn) {
       DCLOG(INFO, dconn) << "Return error reply";
     }
     handler_->set_should_close_after_write(true);
-    if (error_reply(502) != 0) {
-      return -1;
-    }
+    error_reply(502);
     downstream->pop_downstream_connection();
     goto end;
   }
@@ -576,9 +568,7 @@ int HttpsUpstream::downstream_error(DownstreamConnection *dconn, int events) {
   } else {
     status = 502;
   }
-  if (error_reply(status) != 0) {
-    return -1;
-  }
+  error_reply(status);
 
   downstream->pop_downstream_connection();
 
@@ -586,7 +576,7 @@ int HttpsUpstream::downstream_error(DownstreamConnection *dconn, int events) {
   return 0;
 }
 
-int HttpsUpstream::error_reply(unsigned int status_code) {
+void HttpsUpstream::error_reply(unsigned int status_code) {
   auto html = http::create_error_html(status_code);
   auto downstream = get_downstream();
 
@@ -616,8 +606,6 @@ int HttpsUpstream::error_reply(unsigned int status_code) {
 
   downstream->add_response_sent_bodylen(html.size());
   downstream->set_response_state(Downstream::MSG_COMPLETE);
-
-  return 0;
 }
 
 void HttpsUpstream::attach_downstream(std::unique_ptr<Downstream> downstream) {
diff --git a/src/shrpx_https_upstream.h b/src/shrpx_https_upstream.h
index 895f91ef..77f00522 100644
--- a/src/shrpx_https_upstream.h
+++ b/src/shrpx_https_upstream.h
@@ -62,7 +62,7 @@ public:
   void delete_downstream();
   Downstream *get_downstream() const;
   std::unique_ptr<Downstream> pop_downstream();
-  int error_reply(unsigned int status_code);
+  void error_reply(unsigned int status_code);
 
   virtual void pause_read(IOCtrlReason reason);
   virtual int resume_read(IOCtrlReason reason, Downstream *downstream,
diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc
index bca926e3..3ec91e94 100644
--- a/src/shrpx_spdy_upstream.cc
+++ b/src/shrpx_spdy_upstream.cc
@@ -782,7 +782,7 @@ int SpdyUpstream::error_reply(Downstream *downstream,
   if (rv < SPDYLAY_ERR_FATAL) {
     ULOG(FATAL, this) << "spdylay_submit_response() failed: "
                       << spdylay_strerror(rv);
-    DIE();
+    return -1;
   }
 
   return 0;