Simple "Error 13" message improvement

source/client/options_impl.cc

@@ -672,6 +672,7 @@ void OptionsImpl::setNonTrivialDefaults() {
   failure_predicates_["benchmark.http_4xx"] = 0;
   failure_predicates_["benchmark.http_5xx"] = 0;
   failure_predicates_["benchmark.pool_connection_failure"] = 0;
+  failure_predicates_["benchmark.stream_resets"] = 0;
   // Also, fail fast when a remote request source is specified that we can't connect to or otherwise
   // fails.
   failure_predicates_["requestsource.upstream_rq_5xx"] = 0;

source/client/process_impl.cc

@@ -598,7 +598,23 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector<UriP
   StatisticFactoryImpl statistic_factory(options_);
   collector.addResult("global", mergeWorkerStatistics(workers_), counters,
                       total_execution_duration / workers_.size(), first_acquisition_time);
-  return counters.find("sequencer.failed_terminations") == counters.end();
+  if (counters.find("sequencer.failed_terminations") == counters.end()) {
+    return true;
+  } else {
+    ENVOY_LOG(error, "Terminated early because of a failure predicate.");
+    ENVOY_LOG(
+        info,
+        "Check the output for problematic counter values. The default Nighthawk failure predicates "
+        "report failure if (1) Nighthawk could not connect to the target (see "
+        "'benchmark.pool_connection_failure' counter; check the address and port number, and try "
+        "explicitly setting --address-family v4 or v6, especially when using DNS; instead of "
+        "localhost try 127.0.0.1 or ::1 explicitly), (2) the protocol was not supported by the "
+        "target (see 'benchmark.stream_resets' counter; check http/https in the URI, --h2), (3) "
+        "the target returned a 4xx or 5xx HTTP response code (see 'benchmark.http_4xx' and "
+        "'benchmark.http_5xx' counters; check the URI path and the server config), or (4) a custom "
+        "gRPC RequestSource failed. --failure-predicate can be used to relax expectations.");
+    return false;
+  }
 }
 
 bool ProcessImpl::run(OutputCollector& collector) {
@@ -632,7 +648,11 @@ bool ProcessImpl::run(OutputCollector& collector) {
       tracing_uri->resolve(*dispatcher_,
                            Utility::translateFamilyOptionString(options_.addressFamily()));
     }
-  } catch (const UriException&) {
+  } catch (const UriException& ex) {
+    ENVOY_LOG(error,
+              "URI exception (for example, malformed URI syntax, bad "
+              "MultiTarget path, unresolvable host DNS): {}",
+              ex.what());
     return false;
   }
 

source/client/service_impl.cc

@@ -42,7 +42,18 @@ void ServiceImpl::handleExecutionRequest(const nighthawk::client::ExecutionReque
     response.mutable_error_detail()->set_code(grpc::StatusCode::INTERNAL);
     // TODO(https://github.com/envoyproxy/nighthawk/issues/181): wire through error descriptions, so
     // we can do better here.
-    response.mutable_error_detail()->set_message("Unknown failure");
+    response.mutable_error_detail()->set_message(
+        "Unknown failure. See Nighthawk Service logs. Make sure the URI is well formed and the DNS "
+        "name resolves (if applicable). Check the output for problematic counter values. The "
+        "default Nighthawk failure predicates report failure if (1) Nighthawk could not connect to "
+        "the target (see 'benchmark.pool_connection_failure' counter; check the address and port "
+        "number, and try explicitly setting --address-family v4 or v6, especially when using DNS; "
+        "instead of localhost try 127.0.0.1 or ::1 explicitly), (2) the protocol was not supported "
+        "by the target (see 'benchmark.stream_resets' counter; check http/https in the URI, --h2), "
+        "(3) the target returned a 4xx or 5xx HTTP response code (see 'benchmark.http_4xx' and "
+        "'benchmark.http_5xx' counters; check the URI path and the server config), or (4) a custom "
+        "gRPC RequestSource failed. To relax expectations, set explicit failure predicates in the "
+        "benchmark request.");
   }
   *(response.mutable_output()) = output_collector.toProto();
   process.shutdown();

test/options_test.cc

@@ -297,6 +297,7 @@ TEST_P(RequestSourcePluginTestFixture, CreatesOptionsImplWithRequestSourceConfig
   // comparison below.
   EXPECT_EQ(1, command->mutable_failure_predicates()->erase("benchmark.http_4xx"));
   EXPECT_EQ(1, command->mutable_failure_predicates()->erase("benchmark.http_5xx"));
+  EXPECT_EQ(1, command->mutable_failure_predicates()->erase("benchmark.stream_resets"));
   EXPECT_EQ(1, command->mutable_failure_predicates()->erase("requestsource.upstream_rq_5xx"));
 
   // TODO(#433)
@@ -438,6 +439,7 @@ TEST_F(OptionsImplTest, TlsContext) {
   // comparison below.
   EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("benchmark.http_4xx"));
   EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("benchmark.http_5xx"));
+  EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("benchmark.stream_resets"));
   EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("requestsource.upstream_rq_5xx"));
   // TODO(#433)
   OptionsImpl options_from_proto(*cmd);
@@ -500,6 +502,7 @@ TEST_F(OptionsImplTest, MultiTarget) {
   // textual comparison below.
   EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("benchmark.http_4xx"));
   EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("benchmark.http_5xx"));
+  EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("benchmark.stream_resets"));
   EXPECT_EQ(1, cmd->mutable_failure_predicates()->erase("requestsource.upstream_rq_5xx"));
   // TODO(#433)
   OptionsImpl options_from_proto(*cmd);