Repository: streamlit/streamlit
Pull request: 3638/add check script endpoint
**Issue:** https://github.com/streamlit/streamlit/issues/3638

**Description:** 
- add endpoint to verify if the script compiles and runs correctly
- add config option to customise the above endpoint

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.


Problem statement:
Health endpoint for checking that a script compiles and runs correctly
### Problem

Currently there is no way to detect that the streamlit app is actually not running - if there is an error inside the client script (e.g. missing dependency, script error) the application still starts and the /healthz endpoint returns success. 

S4T requires better app monitoring and it would be helpful to detect the applications that cannot start because the above type of error.

### Solution

A new endpoint  will be provided which will return 200 HTTP status code if the script compiles and runs correctly. Alternatively a 503 HTTP status code is returned and the body will contain the error message.

The endpoint by default will be `/check-script`. However, it can be changed by specifying the config option `server.checkScriptEndpoint`

Hints from issue discussion:

Commit message: fix return type

Patch to review:
diff --git a/lib/streamlit/config.py b/lib/streamlit/config.py
index e66d0a8bd3db..18df80f236f7 100644
--- a/lib/streamlit/config.py
+++ b/lib/streamlit/config.py
@@ -589,6 +589,17 @@ def _server_port() -> int:
     return 8501
 
 
+_create_option(
+    "server.checkScriptEndpoint",
+    description="""
+    The endpoint used for checking if a script loads succefully. On success,
+    the endpoint will return a 200 HTTP status code. On failure, the endpoint
+    will return a 503 HTTP status code.
+    """,
+    default_val="check-script",
+    type_=str,
+)
+
 _create_option(
     "server.baseUrlPath",
     description="""
diff --git a/lib/streamlit/server/routes.py b/lib/streamlit/server/routes.py
index 44e83e514ecb..753ab2a5cf73 100644
--- a/lib/streamlit/server/routes.py
+++ b/lib/streamlit/server/routes.py
@@ -171,7 +171,8 @@ def initialize(self, callback):
         self._callback = callback
 
     def get(self):
-        if self._callback():
+        ok, msg = self._callback()
+        if ok:
             self.write("ok")
             self.set_status(200)
 
@@ -186,7 +187,7 @@ def get(self):
         else:
             # 503 = SERVICE_UNAVAILABLE
             self.set_status(503)
-            self.write("unavailable")
+            self.write(msg)
 
 
 class MetricsHandler(_SpecialRequestHandler):
diff --git a/lib/streamlit/server/server.py b/lib/streamlit/server/server.py
index 2300e44b1eed..2339b120fdb1 100644
--- a/lib/streamlit/server/server.py
+++ b/lib/streamlit/server/server.py
@@ -18,10 +18,11 @@
 import socket
 import sys
 import errno
+import types
 import traceback
 import click
 from enum import Enum
-from typing import Any, Dict, Optional, TYPE_CHECKING
+from typing import Any, Dict, Optional, TYPE_CHECKING, Tuple
 
 import tornado.concurrent
 import tornado.gen
@@ -32,6 +33,8 @@
 
 from streamlit import config
 from streamlit import file_util
+from streamlit import magic
+from streamlit import source_util
 from streamlit import util
 from streamlit.config_option import ConfigOption
 from streamlit.forward_msg_cache import ForwardMsgCache
@@ -325,6 +328,8 @@ def _create_app(self):
 
         """
         base = config.get_option("server.baseUrlPath")
+        check_script_endpoint = config.get_option("server.checkScriptEndpoint")
+
         routes = [
             (
                 make_url_path_regex(base, "stream"),
@@ -336,6 +341,11 @@ def _create_app(self):
                 HealthHandler,
                 dict(callback=lambda: self.is_ready_for_browser_connection),
             ),
+            (
+                make_url_path_regex(base, check_script_endpoint),
+                HealthHandler,
+                dict(callback=lambda: self.is_script_loading),
+            ),
             (make_url_path_regex(base, "debugz"), DebugHandler, dict(server=self)),
             (make_url_path_regex(base, "metrics"), MetricsHandler),
             (
@@ -396,8 +406,38 @@ def _set_state(self, new_state):
         self._state = new_state
 
     @property
-    def is_ready_for_browser_connection(self):
-        return self._state not in (State.INITIAL, State.STOPPING, State.STOPPED)
+    def is_ready_for_browser_connection(self) -> Tuple[bool, str]:
+        if self._state not in (State.INITIAL, State.STOPPING, State.STOPPED):
+            return True, "ok"
+
+        return False, "unavailable"
+
+    @property
+    def is_script_loading(self) -> Tuple[bool, str]:
+        try:
+            with source_util.open_python_file(self.script_path) as f:
+                filebody = f.read()
+
+            if config.get_option("runner.magicEnabled"):
+                filebody = magic.add_magic(filebody, self.script_path)
+
+            code = compile(
+                filebody,
+                self.script_path,
+                mode="exec",
+                flags=0,
+                dont_inherit=1,
+                optimize=-1,
+            )
+
+            module = types.ModuleType("__main__")
+            sys.modules["__main__"] = module
+            module.__dict__["__file__"] = self.script_path
+            exec(code, module.__dict__)
+        except BaseException as e:
+            return False, "Error: %s" % str(e)
+
+        return True, "ok"
 
     @property
     def browser_is_connected(self):
diff --git a/lib/tests/streamlit/config_test.py b/lib/tests/streamlit/config_test.py
index 4fc3ffc12ad0..94cfea1b4bbb 100644
--- a/lib/tests/streamlit/config_test.py
+++ b/lib/tests/streamlit/config_test.py
@@ -329,6 +329,7 @@ def test_config_option_keys(self):
                 "server.baseUrlPath",
                 "server.enableCORS",
                 "server.cookieSecret",
+                "server.checkScriptEndpoint",
                 "server.enableWebsocketCompression",
                 "server.enableXsrfProtection",
                 "server.fileWatcherType",
diff --git a/lib/tests/streamlit/server_test.py b/lib/tests/streamlit/server_test.py
index bd8e188f0419..1d7dea598e03 100644
--- a/lib/tests/streamlit/server_test.py
+++ b/lib/tests/streamlit/server_test.py
@@ -17,6 +17,7 @@
 from unittest import mock
 from unittest.mock import MagicMock, patch
 import unittest
+import tempfile
 
 import pytest
 import tornado.testing
@@ -36,6 +37,7 @@
 from streamlit.elements import legacy_data_frame as data_frame
 from streamlit.proto.ForwardMsg_pb2 import ForwardMsg
 from streamlit.server.server import State
+from streamlit.server.server import Server
 from streamlit.server.server import start_listening
 from streamlit.server.server import RetriesExceeded
 from streamlit.server.routes import DebugHandler
@@ -422,7 +424,7 @@ def setUp(self):
         self._is_healthy = True
 
     def is_healthy(self):
-        return self._is_healthy
+        return self._is_healthy, "ok"
 
     def get_app(self):
         return tornado.web.Application(
@@ -593,3 +595,35 @@ def test_message_cache(self):
         # Cache misses
         self.assertEqual(404, self.fetch("/message").code)
         self.assertEqual(404, self.fetch("/message?id=non_existent").code)
+
+
+class ScriptCheckTest(tornado.testing.AsyncTestCase):
+    def test_invalid_script(self):
+        self._check_script_loading(
+            "import streamlit as st\n\nst.deprecatedWrite('test')",
+            False,
+            "Error: module 'streamlit' has no attribute 'deprecatedWrite'",
+        )
+
+    def test_valid_script(self):
+        self._check_script_loading(
+            "import streamlit as st\n\nst.write('test')", True, "ok"
+        )
+
+    def _check_script_loading(self, script, expected_loads, expected_msg):
+        fd, path = tempfile.mkstemp()
+        server = Server(self.io_loop, path, "test command line")
+        try:
+            with os.fdopen(fd, "w") as tmp:
+                # do stuff with temp file
+                tmp.write(script)
+
+            server._on_stopped = mock.MagicMock()  # type: ignore[assignment]
+
+            ok, msg = server.is_script_loading
+            self.assertEqual(expected_loads, ok)
+            self.assertEqual(expected_msg, msg)
+        finally:
+            server.stop()
+            Server._singleton = None
+            os.remove(path)


Code review comments:
diff_hunk: @@ -396,8 +406,38 @@ def _set_state(self, new_state):
         self._state = new_state
 
     @property
-    def is_ready_for_browser_connection(self):
-        return self._state not in (State.INITIAL, State.STOPPING, State.STOPPED)
+    def is_ready_for_browser_connection(self) -> Tuple[bool, str]:
+        if self._state not in (State.INITIAL, State.STOPPING, State.STOPPED):
+            return True, "ok"
+
+        return False, "unavailable"
+
+    @property
+    def is_script_loading(self) -> Tuple[bool, str]:
+        try:
+            with source_util.open_python_file(self.script_path) as f:
+                filebody = f.read()
+
+            if config.get_option("runner.magicEnabled"):
+                filebody = magic.add_magic(filebody, self.script_path)
+
+            code = compile(
+                filebody,
+                self.script_path,
+                mode="exec",
+                flags=0,
+                dont_inherit=1,
+                optimize=-1,
+            )
+
+            module = types.ModuleType("__main__")
+            sys.modules["__main__"] = module
+            module.__dict__["__file__"] = self.script_path
+            exec(code, module.__dict__)
path: lib/streamlit/server/server.py
text: @user1:
I'm a bit concerned about what might happen when actually trying to run the script here.

Because running a streamlit script requires a decent amount of machinery to be in place, we use an internal global variable `_is_running_with_streamlit` to ensure that a script is essentially a big no-op when a user does something like running it directly (that is, runs something like `python3 my_script.py`).

This flag gets set to true from within `cli.py`. What could be strange about running a script from here is that the flag may be set when the script is run from here, but in this context, I don't believe most of the machinery required to actually run many of the `st.*` commands will be available.

I'm not exactly sure what'll happen when running the script in `exec`, so it would be useful to check the value of `st._is_running_with_streamlit` from within the script when it's run via this endpoint. If the flag is set to True, then it's possible that we'll see some weird/undefined behavior here.

@user2:
I suspect the more-correct thing to do here is to actually kick of a `ScriptRunner` execution of the script, rather than just `exec`'ing it "raw". That way the script is actually run "within Streamlit" (and if the definition of "within Streamlit" changes in ScriptRunner, it won't need to also change here).

The downside is that using ScriptRunner is a bigger pain than what you've already done (because it requires some inputs, and also it functions asynchronously on another thread that it creates). But we could definitely help with that integration!

diff_hunk: @@ -171,7 +171,8 @@ def initialize(self, callback):
         self._callback = callback
 
     def get(self):
-        if self._callback():
+        ok, msg = self._callback()
+        if ok:
             self.write("ok")
path: lib/streamlit/server/routes.py
text: @user1:
nit: should change this to `self.write(msg)` to match the 503 case below

diff_hunk: @@ -396,8 +406,38 @@ def _set_state(self, new_state):
         self._state = new_state
 
     @property
-    def is_ready_for_browser_connection(self):
-        return self._state not in (State.INITIAL, State.STOPPING, State.STOPPED)
+    def is_ready_for_browser_connection(self) -> Tuple[bool, str]:
+        if self._state not in (State.INITIAL, State.STOPPING, State.STOPPED):
+            return True, "ok"
+
+        return False, "unavailable"
+
+    @property
+    def is_script_loading(self) -> Tuple[bool, str]:
+        try:
+            with source_util.open_python_file(self.script_path) as f:
+                filebody = f.read()
+
+            if config.get_option("runner.magicEnabled"):
+                filebody = magic.add_magic(filebody, self.script_path)
+
+            code = compile(
+                filebody,
+                self.script_path,
+                mode="exec",
+                flags=0,
+                dont_inherit=1,
+                optimize=-1,
+            )
+
+            module = types.ModuleType("__main__")
+            sys.modules["__main__"] = module
+            module.__dict__["__file__"] = self.script_path
+            exec(code, module.__dict__)
+        except BaseException as e:
+            return False, "Error: %s" % str(e)
path: lib/streamlit/server/server.py
text: @user1:
Nit: use string interpolation: `return False, f"Error: {str(e)}"` (we don't do this consistently thru the codebase because we originally targeted old Pythons, but now that we're 3.6+, we should be for new code.)

diff_hunk: @@ -396,8 +406,38 @@ def _set_state(self, new_state):
         self._state = new_state
 
     @property
-    def is_ready_for_browser_connection(self):
-        return self._state not in (State.INITIAL, State.STOPPING, State.STOPPED)
+    def is_ready_for_browser_connection(self) -> Tuple[bool, str]:
+        if self._state not in (State.INITIAL, State.STOPPING, State.STOPPED):
+            return True, "ok"
+
+        return False, "unavailable"
+
+    @property
+    def is_script_loading(self) -> Tuple[bool, str]:
path: lib/streamlit/server/server.py
text: @user1:
A few nits:

- Feels like maybe this shouldn't be a property, since it does an (unbounded!) amount of computation, and doesn't cache the result.
- The name is confusing to me. What about something like: `does_script_execute_without_error()` or something?
- Could use a docstring, since it's doing a non-obvious thing:

```
"""Load and execute the app's script to verify it runs without an error.

Returns
-------
[True, "ok"] if the script completes without error, or [False, err_msg] if the script raises an exception.
"""
```