Review exceptions thrown by github-ci API functions

8 files changed, 168 insertions, 37 deletions

diff --git a/mod/mod-ci-github-gh.cxx b/mod/mod-ci-github-gh.cxx
index 6ab93d7..a1e4d53 100644
--- a/mod/mod-ci-github-gh.cxx
+++ b/mod/mod-ci-github-gh.cxx
@@ -7,10 +7,19 @@
 
 namespace brep
 {
+  [[noreturn]] static void
+  throw_json (const json::parser& p, const string& m)
+  {
+    throw json::invalid_json_input (
+      p.input_name,
+      p.line (), p.column (), p.position (),
+      m);
+  }
+
   // Return the GitHub check run status corresponding to a build_state.
   //
   string
-  gh_to_status (build_state st)
+  gh_to_status (build_state st) noexcept
   {
     // Just return by value (small string optimization).
     //
@@ -102,10 +111,7 @@ namespace brep
   [[noreturn]] static void
   missing_member (const json::parser& p, const char* o, const char* m)
   {
-    throw json::invalid_json_input (
-      p.input_name,
-      p.line (), p.column (), p.position (),
-      o + string (" object is missing member '") + m + '\'');
+    throw_json (p, o + string (" object is missing member '") + m + '\'');
   }
 
   using event = json::event;
@@ -577,7 +583,21 @@ namespace brep
       };
 
       if      (c (tk, "token"))      token = p.next_expect_string ();
-      else if (c (ea, "expires_at")) expires_at = gh_from_iso8601 (p.next_expect_string ());
+      else if (c (ea, "expires_at"))
+      {
+        string v (p.next_expect_string ());
+
+        try
+        {
+          expires_at = gh_from_iso8601 (v);
+        }
+        catch (const invalid_argument& e)
+        {
+          throw_json (p,
+                      "invalid IAT expires_at value '" + v +
+                        "': " + e.what ());
+        }
+      }
       else p.next_expect_value_skip ();
     }
 
@@ -603,15 +623,37 @@ namespace brep
   string
   gh_to_iso8601 (timestamp t)
   {
-    return butl::to_string (t,
-                            "%Y-%m-%dT%TZ",
-                            false /* special */,
-                            false /* local */);
+    try
+    {
+      return butl::to_string (t,
+                              "%Y-%m-%dT%TZ",
+                              false /* special */,
+                              false /* local */);
+    }
+    catch (const system_error& e)
+    {
+      throw runtime_error (
+        string ("failed to convert timestamp to ISO 8601 string: ") +
+        e.what ());
+    }
   }
 
   timestamp
   gh_from_iso8601 (const string& s)
   {
-    return butl::from_string (s.c_str (), "%Y-%m-%dT%TZ", false /* local */);
+    try
+    {
+      // @@ TMP butl::from_string()'s comment says it also throws
+      //    invalid_argument but that seems to be false.
+      //
+      return butl::from_string (s.c_str (),
+                                "%Y-%m-%dT%TZ",
+                                false /* local */);
+    }
+    catch (const system_error& e)
+    {
+      throw invalid_argument ("invalid ISO 8601 timestamp value '" + s +
+                              "': " + e.what ());
+    }
   }
 }
diff --git a/mod/mod-ci-github-gh.hxx b/mod/mod-ci-github-gh.hxx
index 6ede697..5fff2bc 100644
--- a/mod/mod-ci-github-gh.hxx
+++ b/mod/mod-ci-github-gh.hxx
@@ -107,7 +107,7 @@ namespace brep
   // Return the GitHub check run status corresponding to a build_state.
   //
   string
-  gh_to_status (build_state st);
+  gh_to_status (build_state st) noexcept;
 
   // Return the build_state corresponding to a GitHub check run status
   // string. Throw invalid_argument if the passed status was invalid.
@@ -118,6 +118,9 @@ namespace brep
   // If warning_success is true, then map result_status::warning to SUCCESS
   // and to FAILURE otherwise.
   //
+  // Throw invalid_argument in case of unsupported result_status value
+  // (currently skip, interrupt).
+  //
   string
   gh_to_conclusion (result_status, bool warning_success);
 
@@ -211,9 +214,13 @@ namespace brep
     gh_installation_access_token () = default;
   };
 
+  // Throw runtime_error if the conversion fails.
+  //
   string
   gh_to_iso8601 (timestamp);
 
+  // Throw invalid_argument if the conversion fails.
+  //
   timestamp
   gh_from_iso8601 (const string&);
 
diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx
index fa701a8..a0a0d6b 100644
--- a/mod/mod-ci-github-gq.cxx
+++ b/mod/mod-ci-github-gq.cxx
@@ -163,6 +163,8 @@ namespace brep
   // Parse a response to a check_run GraphQL mutation such as `createCheckRun`
   // or `updateCheckRun`.
   //
+  // Throw invalid_json_input.
+  //
   // Example response (only the part we need to parse here):
   //
   //  {
@@ -229,7 +231,7 @@ namespace brep
   gq_mutate_check_runs (const basic_mark& error,
                         vector<check_run>& crs,
                         const string& iat,
-                        string rq) noexcept
+                        string rq)
   {
     vector<gh_check_run> rcrs;
 
@@ -302,7 +304,7 @@ namespace brep
         error << "failed to mutate check runs: error HTTP response status "
               << sc;
     }
-    catch (const json::invalid_json_input& e)
+    catch (const json::invalid_json_input& e) // struct resp (via github_post())
     {
       // Note: e.name is the GitHub API endpoint.
       //
@@ -310,16 +312,16 @@ namespace brep
             << e.line << ", column: " << e.column << ", byte offset: "
             << e.position << ", error: " << e;
     }
-    catch (const invalid_argument& e)
+    catch (const invalid_argument& e) // github_post()
     {
       error << "malformed header(s) in response: " << e;
     }
-    catch (const system_error& e)
+    catch (const system_error& e) // github_post()
     {
       error << "unable to mutate check runs (errno=" << e.code () << "): "
             << e.what ();
     }
-    catch (const runtime_error& e) // From gq_parse_response_check_runs().
+    catch (const runtime_error& e) // gq_parse_response_check_runs()
     {
       // GitHub response contained error(s) (could be ours or theirs at this
       // point).
@@ -361,6 +363,9 @@ namespace brep
   // The details URL argument (`du`) can be empty for queued but not for the
   // other states.
   //
+  // Throw invalid_argument if any of the observed check run members are not
+  // valid GraphQL values (string, enum, etc).
+  //
   static string
   gq_mutation_create_check_runs (const string& ri,           // Repository ID
                                  const string& hs,           // Head SHA
@@ -422,6 +427,9 @@ namespace brep
   // The details URL argument (`du`) can be empty for queued but not for the
   // other states.
   //
+  // Throw invalid_argument if any of the arguments or observed check run
+  // members are not valid GraphQL values (string, enum, etc).
+  //
   static string
   gq_mutation_create_check_run (const string& ri,           // Repository ID
                                 const string& hs,           // Head SHA
@@ -484,6 +492,9 @@ namespace brep
   // because GitHub does not allow updating a check run to completed without a
   // conclusion.
   //
+  // Throw invalid_argument if any of the arguments are invalid values (of
+  // GraphQL types or otherwise).
+  //
   static string
   gq_mutation_update_check_run (const string& ri,           // Repository ID.
                                 const string& ni,           // Node ID.
@@ -505,8 +516,17 @@ namespace brep
        << "  status: "       << gq_enum (st);
     if (sa)
     {
-      os                                                          << '\n';
-      os << "  startedAt: " << gq_str (gh_to_iso8601 (*sa));
+      try
+      {
+        os                                                        << '\n';
+        os << "  startedAt: " << gq_str (gh_to_iso8601 (*sa));
+      }
+      catch (const runtime_error& e)
+      {
+        throw invalid_argument ("invalid started_at value " +
+                                to_string (system_clock::to_time_t (*sa)) +
+                                ": " + e.what ());
+      }
     }
     if (du)
     {
@@ -634,6 +654,8 @@ namespace brep
 
   // Serialize a GraphQL query that fetches a pull request from GitHub.
   //
+  // Throw invalid_argument if the node id is not a valid GraphQL string.
+  //
   static string
   gq_query_pr_mergeability (const string& nid)
   {
@@ -658,6 +680,8 @@ namespace brep
                                         const string& iat,
                                         const string& nid)
   {
+    // Let invalid_argument from gq_query_pr_mergeability() propagate.
+    //
     string rq (gq_serialize_request (gq_query_pr_mergeability (nid)));
 
     try
@@ -760,7 +784,7 @@ namespace brep
         error << "failed to fetch pull request: error HTTP response status "
               << sc;
     }
-    catch (const json::invalid_json_input& e)
+    catch (const json::invalid_json_input& e) // struct resp (via github_post())
     {
       // Note: e.name is the GitHub API endpoint.
       //
@@ -768,16 +792,16 @@ namespace brep
             << e.line << ", column: " << e.column << ", byte offset: "
             << e.position << ", error: " << e;
     }
-    catch (const invalid_argument& e)
+    catch (const invalid_argument& e) // github_post()
     {
       error << "malformed header(s) in response: " << e;
     }
-    catch (const system_error& e)
+    catch (const system_error& e) // github_post()
     {
       error << "unable to fetch pull request (errno=" << e.code () << "): "
             << e.what ();
     }
-    catch (const runtime_error& e) // From response type's parsing constructor.
+    catch (const runtime_error& e) // struct resp
     {
       // GitHub response contained error(s) (could be ours or theirs at this
       // point).
diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx
index 7c564d7..86ab859 100644
--- a/mod/mod-ci-github-gq.hxx
+++ b/mod/mod-ci-github-gq.hxx
@@ -24,6 +24,8 @@ namespace brep
   // `check_runs` with the new data (node id and state_synced). Return false
   // and issue diagnostics if the request failed.
   //
+  // Throw invalid_argument.
+  //
   // Note that creating a check_run named `foo` will effectively replace any
   // existing check_runs with that name. They will still exist on the GitHub
   // servers but GitHub will only consider the latest one (for display in the
@@ -40,6 +42,8 @@ namespace brep
   // data (node id, state, and state_synced). Return false and issue
   // diagnostics if the request failed.
   //
+  // Throw invalid_argument.
+  //
   // If the details_url is absent GitHub will use the app's homepage.
   //
   // The gq_built_result is required if the build_state is built because
@@ -66,6 +70,8 @@ namespace brep
   // Update a check run on GitHub. Update `cr` with the new data (state and
   // state_synced). Return false and issue diagnostics if the request failed.
   //
+  // Throw invalid_argument.
+  //
   // Note that GitHub allows any state transitions except from built (but
   // built to built is allowed). The latter case is signalled by setting the
   // check_run state_synced member to false and the state member to built.
@@ -99,6 +105,8 @@ namespace brep
   // Issue diagnostics and return absent if the request failed (which means it
   // will be treated by the caller as still being generated).
   //
+  // Throw invalid_argument if the node id is invalid.
+  //
   // Note that the first request causes GitHub to start preparing the test
   // merge commit.
   //
diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx
index 68a1426..d3071b2 100644
--- a/mod/mod-ci-github-service-data.cxx
+++ b/mod/mod-ci-github-service-data.cxx
@@ -10,6 +10,15 @@ namespace brep
 {
   using event = json::event;
 
+  [[noreturn]] static void
+  throw_json (json::parser& p, const string& m)
+  {
+    throw json::invalid_json_input (
+      p.input_name,
+      p.line (), p.column (), p.position (),
+      m);
+  }
+
   service_data::
   service_data (const string& json)
   {
@@ -32,11 +41,7 @@ namespace brep
       if      (v == "local")  kind = local;
       else if (v == "remote") kind = remote;
       else
-      {
-        throw json::invalid_json_input (
-          p.input_name, p.line (), p.column (), p.position (),
-          "invalid service data kind: '" + v + '\'');
-      }
+        throw_json (p, "invalid service data kind: '" + v + '\'');
     }
 
     pre_check = p.next_expect_member_boolean<bool> ("pre_check");
@@ -44,7 +49,7 @@ namespace brep
 
     warning_success = p.next_expect_member_boolean<bool> ("warning_success");
 
-    // Installation access token.
+    // Installation access token (IAT).
     //
     p.next_expect_name ("installation_access");
     installation_access = gh_installation_access_token (p);
@@ -79,7 +84,16 @@ namespace brep
           nid = *v;
       }
 
-      build_state s (to_build_state (p.next_expect_member_string ("state")));
+      build_state s;
+      try
+      {
+        s = to_build_state (p.next_expect_member_string ("state"));
+      }
+      catch (const invalid_argument& e)
+      {
+        throw_json (p, e.what ());
+      }
+
       bool ss (p.next_expect_member_boolean<bool> ("state_synced"));
 
       optional<result_status> rs;
@@ -87,7 +101,14 @@ namespace brep
         string* v (p.next_expect_member_string_null ("status"));
         if (v != nullptr)
         {
-          rs = bbot::to_result_status (*v);
+          try
+          {
+            rs = bbot::to_result_status (*v);
+          }
+          catch (const invalid_argument& e)
+          {
+            throw_json (p, e.what ());
+          }
           assert (s == build_state::built);
         }
       }
@@ -186,11 +207,28 @@ namespace brep
 
     s.member ("warning_success", warning_success);
 
-    // Installation access token.
+    // Installation access token (IAT).
     //
     s.member_begin_object ("installation_access");
     s.member ("token", installation_access.token);
-    s.member ("expires_at", gh_to_iso8601 (installation_access.expires_at));
+
+    // IAT expires_at timestamp.
+    //
+    {
+      string v;
+      try
+      {
+        v = gh_to_iso8601 (installation_access.expires_at);
+      }
+      catch (const runtime_error& e)
+      {
+        throw invalid_argument ("invalid IAT expires_at value " +
+                                to_string (system_clock::to_time_t (
+                                  installation_access.expires_at)));
+      }
+      s.member ("expires_at", move (v));
+    }
+
     s.end_object ();
 
     s.member ("installation_id", installation_id);
@@ -232,7 +270,7 @@ namespace brep
       if (cr.status)
       {
         assert (cr.state == build_state::built);
-        s.value (to_string (*cr.status));
+        s.value (to_string (*cr.status)); // Doesn't throw.
       }
       else
         s.value (nullptr);
diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx
index cabd19a..776ec8d 100644
--- a/mod/mod-ci-github-service-data.hxx
+++ b/mod/mod-ci-github-service-data.hxx
@@ -139,6 +139,9 @@ namespace brep
     //
     // Throw invalid_argument if the schema version is not supported.
     //
+    // Throw invalid_argument (invalid_json_input) in case of malformed JSON
+    // or any invalid values.
+    //
     explicit
     service_data (const string& json);
 
@@ -179,6 +182,11 @@ namespace brep
 
     // Serialize to JSON.
     //
+    // Throw invalid_argument if any values are invalid.
+    //
+    // May also throw invalid_json_output but that would be a programming
+    // error.
+    //
     string
     json () const;
   };
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 46cd0cc..c703f32 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -2996,6 +2996,8 @@ namespace brep
       //
       iat.expires_at -= chrono::minutes (5);
     }
+    // gh_installation_access_token (via github_post())
+    //
     catch (const json::invalid_json_input& e)
     {
       // Note: e.name is the GitHub API endpoint.
@@ -3005,12 +3007,12 @@ namespace brep
             << e.position << ", error: " << e;
       return nullopt;
     }
-    catch (const invalid_argument& e)
+    catch (const invalid_argument& e) // github_post()
     {
       error << "malformed header(s) in response: " << e;
       return nullopt;
     }
-    catch (const system_error& e)
+    catch (const system_error& e) // github_post()
     {
       error << "unable to get installation access token (errno=" << e.code ()
             << "): " << e.what ();
diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx
index b88d5e4..c38462a 100644
--- a/mod/mod-ci-github.hxx
+++ b/mod/mod-ci-github.hxx
@@ -107,7 +107,9 @@ namespace brep
     optional<string>
     generate_jwt (const basic_mark& trace, const basic_mark& error) const;
 
-    // Authenticate to GitHub as an app installation.
+    // Authenticate to GitHub as an app installation. Return the installation
+    // access token (IAT). Issue diagnostics and return nullopt if something
+    // goes wrong.
     //
     optional<gh_installation_access_token>
     obtain_installation_access_token (uint64_t install_id,