Repository: SCons/scons

Issue:
SCons may convert another exception to BuildError incorrectly
An internal SCons routine, `SCons.Errors.convert_to_BuildError`, attempts to synthesize a `BuildError` exception from whatever exception may have been raised when the Taskmaster executes an action.  This can go wrong in certain circumstances.  The docstring for the `BuildError` class contains these comments:

> `errstr`: a description of the error message                     
> `status`: the return code of the action that caused the build error.  Must be set to a non-zero value even if the build error is not due   to an action returning a non-zero returned code. 

In the case reported in the linked thread, the Jinja templating engine (used in-process) raised a `jinja2.exceptions.TemplateNotFound` exception, which is initialized in a kind of odd way (separate topic), where the three important exception fields `filename`, `strerror` and `errno` are all set to `None`. The conversion routine then tries this (`status` contains the captured exception at this point):
```py
    filename = getattr(status, 'filename', None) 
    strerror = getattr(status, 'strerror', str(status)) 
    errno = getattr(status, 'errno', 2) 
```
before stuffing those values into the `BuildError` instance.  As written, `strerror` and `errno` will remain `None` if they were `None` in the original exception, which is not the intent of the routine, as described in the docstring (the getattr defaults only apply if the attributes are not present). The effect is that the exception appears to be "swallowed". SCons should be more tolerant of this, and get the defaults in if the fields were `None`.

* Mailing list reference:  https://pairlist4.pair.net/pipermail/scons-users/2024-May/009349.html
* Version of SCons: 4.7.0


Simple repro:
```py
import jinja2

def small_builder(target, source, env):
    raise jinja2.exceptions.TemplateNotFound("You should see this message")

env=Environment()
env.Command(['dummy/TEST.COMPLETE'], [], small_builder, chdir=0)
```


Discussion:

Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index 8815a2061..0a36b9405 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -63,6 +63,9 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       old Python 2-only code block in a test.
     - scons-time tests now supply a "filter" argument to tarfile.extract
       to quiet a warning which was added in Python 3.13 beta 1.
+    - Improved the conversion of a "foreign" exception from an action
+      into BuildError by making sure our defaults get applied even in
+      corner cases. Fixes #4530.
     - Restructured API docs build (Sphinx) so main module contents appear
       on a given page *before* the submodule docs, not after. Also
       tweaked the Util package doc build so it's structured more like the
diff --git a/RELEASE.txt b/RELEASE.txt
index 614333986..684121070 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -49,6 +49,9 @@ FIXES
 -----
 
 - OSErrors are now no longer hidden during the execution of Actions.
+- Improved the conversion of a "foreign" exception from an action
+  into BuildError by making sure our defaults get applied even in
+  corner cases. Fixes Issue #4530
 
 IMPROVEMENTS
 ------------
diff --git a/SCons/Errors.py b/SCons/Errors.py
index e489c58a0..a2efc9708 100644
--- a/SCons/Errors.py
+++ b/SCons/Errors.py
@@ -177,8 +177,12 @@ def convert_to_BuildError(status, exc_info=None):
         # (for example, failure to create the directory in which the
         # target file will appear).
         filename = getattr(status, 'filename', None)
-        strerror = getattr(status, 'strerror', str(status))
-        errno = getattr(status, 'errno', 2)
+        strerror = getattr(status, 'strerror', None)
+        if strerror is None:
+            strerror = str(status)
+        errno = getattr(status, 'errno', None)
+        if errno is None:
+            errno = 2
 
         buildError = BuildError(
             errstr=strerror,


Test patch:
diff --git a/SCons/ErrorsTests.py b/SCons/ErrorsTests.py
index 9743f0db3..4241750e3 100644
--- a/SCons/ErrorsTests.py
+++ b/SCons/ErrorsTests.py
@@ -46,7 +46,7 @@ class ErrorsTestCase(unittest.TestCase):
             assert e.command == "c"
 
         try:
-            raise SCons.Errors.BuildError("n", "foo", 57, 3, "file", 
+            raise SCons.Errors.BuildError("n", "foo", 57, 3, "file",
                                           "e", "a", "c", (1,2,3))
         except SCons.Errors.BuildError as e:
             assert e.errstr == "foo", e.errstr
@@ -96,26 +96,48 @@ class ErrorsTestCase(unittest.TestCase):
             assert e.node == "node"
 
     def test_convert_EnvironmentError_to_BuildError(self) -> None:
-        """Test the convert_to_BuildError function on SConsEnvironmentError
-        exceptions.
-        """
+        """Test convert_to_BuildError on SConsEnvironmentError."""
         ee = SCons.Errors.SConsEnvironmentError("test env error")
         be = SCons.Errors.convert_to_BuildError(ee)
-        assert be.errstr == "test env error"
-        assert be.status == 2
-        assert be.exitstatus == 2
-        assert be.filename is None
+        with self.subTest():
+            self.assertEqual(be.errstr, "test env error")
+        with self.subTest():
+            self.assertEqual(be.status, 2)
+        with self.subTest():
+            self.assertEqual(be.exitstatus, 2)
+        with self.subTest():
+            self.assertIsNone(be.filename)
 
     def test_convert_OSError_to_BuildError(self) -> None:
-        """Test the convert_to_BuildError function on OSError
-        exceptions.
-        """
+        """Test convert_to_BuildError on OSError."""
         ose = OSError(7, 'test oserror')
         be = SCons.Errors.convert_to_BuildError(ose)
-        assert be.errstr == 'test oserror'
-        assert be.status == 7
-        assert be.exitstatus == 2
-        assert be.filename is None
+        with self.subTest():
+            self.assertEqual(be.errstr, 'test oserror')
+        with self.subTest():
+            self.assertEqual(be.status, 7)
+        with self.subTest():
+            self.assertEqual(be.exitstatus, 2)
+        with self.subTest():
+            self.assertIsNone(be.filename)
+
+    def test_convert_phony_OSError_to_BuildError(self) -> None:
+        """Test convert_to_BuildError on OSError with defaults."""
+        class PhonyException(OSError):
+            def __init__(self, name):
+                OSError.__init__(self, name)  # most fields will default to None
+                self.name = name
+
+        ose = PhonyException("test oserror")
+        be = SCons.Errors.convert_to_BuildError(ose)
+        with self.subTest():
+            self.assertEqual(be.errstr, 'test oserror')
+        with self.subTest():
+            self.assertEqual(be.status, 2)
+        with self.subTest():
+            self.assertEqual(be.exitstatus, 2)
+        with self.subTest():
+            self.assertIsNone(be.filename)
 
 
 if __name__ == "__main__":