[gjs/ewlsh/top-level-await-mainloop] Refactor exit code handling for async exceptions




commit 8f2f2ebf3fa368d58dd23989f0ffcfa2736fa58c
Author: Evan Welsh <contact evanwelsh com>
Date:   Thu Jan 13 22:19:19 2022 -0800

    Refactor exit code handling for async exceptions
    
    With an implicit mainloop our exit code handling
    needs to handle exceptions which are pending
    after the initial script has synchronously returned.
    
    This means we can't rely on "ok" being false to indicate
    a program is exiting with an error.

 gjs/context-private.h |  5 ++--
 gjs/context.cpp       | 68 ++++++++++++++++++++++++++++++---------------------
 2 files changed, 43 insertions(+), 30 deletions(-)
---
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 48394b4d..b2cbf50d 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -139,8 +139,9 @@ class GjsContextPrivate : public JS::JobQueue {
     void start_draining_job_queue(void);
     void stop_draining_job_queue(void);
 
-    uint8_t handle_exit_code(const char* type, const char* identifier,
-                             GError** error);
+    bool handle_exit_code(bool has_sync_error, const char* type,
+                          const char* identifier, uint8_t* exit_code,
+                          GError** error);
     [[nodiscard]] bool auto_profile_enter(void);
     void auto_profile_exit(bool status);
 
diff --git a/gjs/context.cpp b/gjs/context.cpp
index dc0573e6..9fed1f43 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -1216,31 +1216,49 @@ void GjsContextPrivate::auto_profile_exit(bool auto_profile) {
         gjs_profiler_stop(m_profiler);
 }
 
-uint8_t GjsContextPrivate::handle_exit_code(const char* type,
-                                            const char* identifier,
-                                            GError** error) {
+bool GjsContextPrivate::handle_exit_code(bool has_sync_error, const char* type,
+                                         const char* identifier,
+                                         uint8_t* exit_code, GError** error) {
     uint8_t code;
     if (should_exit(&code)) {
         /* exit_status_p is public API so can't be changed, but should be
          * uint8_t, not int */
         g_set_error(error, GJS_ERROR, GJS_ERROR_SYSTEM_EXIT,
                     "Exit with code %d", code);
-        return code;  // Don't log anything
+
+        *exit_code = code;
+        return false;  // Don't log anything
     }
-    if (!JS_IsExceptionPending(m_cx)) {
-        g_critical("%s %s terminated with an uncatchable exception", type,
-                   identifier);
-        g_set_error(error, GJS_ERROR, GJS_ERROR_FAILED,
-                    "%s %s terminated with an uncatchable exception", type,
-                    identifier);
-    } else {
+
+    // Once the main loop exits an exception could
+    // be pending even if the script returned
+    // true synchronously
+    if (JS_IsExceptionPending(m_cx)) {
         g_set_error(error, GJS_ERROR, GJS_ERROR_FAILED,
                     "%s %s threw an exception", type, identifier);
+        gjs_log_exception_uncaught(m_cx);
+
+        *exit_code = 1;
+        return false;
+    }
+
+    // Assume success if no error was thrown and should exit isn't
+    // set
+    if (!has_sync_error) {
+        *exit_code = 0;
+        return true;
     }
 
+    g_critical("%s %s terminated with an uncatchable exception", type,
+               identifier);
+    g_set_error(error, GJS_ERROR, GJS_ERROR_FAILED,
+                "%s %s terminated with an uncatchable exception", type,
+                identifier);
+
     gjs_log_exception_uncaught(m_cx);
     /* No exit code from script, but we don't want to exit(0) */
-    return 1;
+    *exit_code = 1;
+    return false;
 }
 
 bool GjsContextPrivate::eval(const char* script, ssize_t script_len,
@@ -1268,25 +1286,21 @@ bool GjsContextPrivate::eval(const char* script, ssize_t script_len,
 
     auto_profile_exit(auto_profile);
 
-    if (!ok) {
-        *exit_status_p = handle_exit_code("Script", filename, error);
-        return false;
-    }
+    uint8_t out_code;
+    ok = handle_exit_code(!ok, "Script", filename, &out_code, error);
 
     if (exit_status_p) {
-        if (retval.isInt32()) {
+        if (ok && retval.isInt32()) {
             int code = retval.toInt32();
             gjs_debug(GJS_DEBUG_CONTEXT,
                       "Script returned integer code %d", code);
             *exit_status_p = code;
         } else {
-            // Assume success if no integer was returned and should exit isn't
-            // set
-            *exit_status_p = 0;
+            *exit_status_p = out_code;
         }
     }
 
-    return true;
+    return ok;
 }
 
 bool GjsContextPrivate::eval_module(const char* identifier,
@@ -1331,14 +1345,12 @@ bool GjsContextPrivate::eval_module(const char* identifier,
 
     auto_profile_exit(auto_profile);
 
-    if (!ok) {
-        *exit_status_p = handle_exit_code("Module", identifier, error);
-        return false;
-    }
+    uint8_t out_code;
+    ok = handle_exit_code(!ok, "Module", identifier, &out_code, error);
+    if (exit_status_p)
+        *exit_status_p = out_code;
 
-    /* Assume success if no errors were thrown or exit code set. */
-    *exit_status_p = 0;
-    return true;
+    return ok;
 }
 
 bool GjsContextPrivate::register_module(const char* identifier, const char* uri,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]