Minor improvements in error handling, primary OOM

.github/workflows/basic.yml

@@ -169,8 +169,16 @@ jobs:
         run: >
           python3 -m pytest src/projects/spades/pipeline/spades_pipeline/tests/test_error_codes.py
 
+      - name: '📦 Unpack package for integration tests'
+        run: |
+          mkdir -p $INSTALL_DIR
+          tar -zxf $BUILD_DIR/$PKG_LINUX.tar.gz -C $INSTALL_DIR
+          echo "SPADES_PKG_DIR=$INSTALL_DIR/$(ls $INSTALL_DIR)" >> $GITHUB_ENV
+
       - name: '🔎 Python integration tests'
         working-directory: ${{github.workspace}}
+        env:
+          SPADES_BUILD_DIR: ${{env.SPADES_PKG_DIR}}
         run: >
           python3 -m pytest src/test/integration/test_error_codes.py
 

ext/src/mimalloc/src/options.c

@@ -13,6 +13,7 @@ terms of the MIT license. A copy of the license can be found in the file
 #include <string.h> // strncpy, strncat, strlen, strstr
 #include <ctype.h>  // toupper
 #include <stdarg.h>
+#include <unistd.h> // _exit
 
 #ifdef _MSC_VER
 #pragma warning(disable:4996)   // strncpy, strncat
@@ -431,7 +432,9 @@ static void mi_error_default(int err) {
     _mi_fprintf(NULL, NULL, "%10s: current: %lu, peak: %lu\n", "rss", current_rss, peak_rss);
     _mi_fprintf(NULL, NULL, "%10s: current: %lu, peak: %lu\n", "commit", current_commit, peak_commit);
 
-    abort();
+    // abort();
+    // to comply with SPAdes error codes
+    _exit(68);
   }
 #endif
 }

src/projects/spades/pipeline/spades.py

@@ -711,7 +711,7 @@ def main(args):
         if exc_type == SystemExit:
             # If SystemExit carries a numeric code, propagate it; otherwise map to a general error
             try:
-                code = int(exc_value)
+                code = int(str(exc_value))
                 sys.exit(code)
             except Exception:
                 support.error(str(exc_value), exit_code=support.ErrorCode.GeneralError)

src/projects/spades/pipeline/spades_pipeline/support.py

@@ -45,27 +45,31 @@ class ErrorCode(Enum):
     InvalidParameter = 67
     MemoryLimitExceeded = 68
 
+    @classmethod
+    def _missing_(cls, value):
+        return cls.GeneralError
+
     def user_end_error(self):
         return 64 <= self.value <= 127
 
 
-def report_issue_error_message(binary_name):
-    return ["In case you have troubles running %s, you can report an issue on our GitHub repository github.com/ablab/spades\n" % binary_name,
-            "Please provide us with params.txt and %s.log files from the output directory." % binary_name.lower()]
+def report_issue_error_message(binary_name, error_code):
+    return ["%s finished with the following error code: %d.\n" % (binary_name, error_code),
+            "If the reason for this error is unclear, you can report an issue on our GitHub repository github.com/ablab/spades\n",
+            "Please provide us with params.txt and %s.log files from the output directory.\n" % binary_name.lower() ]
 
 
-def no_report_error_message(binary_name):
-    return ["%s finished with the following error code. Please, check the error message above." % binary_name]
+def no_report_error_message(binary_name, error_code):
+    return ["%s finished with the following error code: %d. Please, check the error message above.\n" % (binary_name, error_code)]
 
 
 def error(err_str, logger_instance=None, prefix=SPADES_PY_ERROR_MESSAGE, exit_code:ErrorCode=ErrorCode.GeneralError):
     binary_name = "SPAdes"
 
     if exit_code.user_end_error():
-        error_message = no_report_error_message(binary_name)
+        error_message = no_report_error_message(binary_name, exit_code.value)
     else:
-        error_message = report_issue_error_message(binary_name)
-
+        error_message = report_issue_error_message(binary_name, exit_code.value)
     if logger_instance:
         logger_instance.error("\n\n%s %s" % (prefix, err_str))
         log_warnings(logger_instance, with_error=True)
@@ -78,7 +82,7 @@ def error(err_str, logger_instance=None, prefix=SPADES_PY_ERROR_MESSAGE, exit_co
         sys.stderr.flush()
     if current_tmp_dir and os.path.isdir(current_tmp_dir):
         shutil.rmtree(current_tmp_dir)
-    sys.exit(exit_code.value)
+    sys.exit(int(exit_code.value))
 
 
 def warning(warn_str, logger_instance=None, prefix=SPADES_PY_WARN_MESSAGE):

src/projects/spades/pipeline/spades_pipeline/tests/test_error_codes.py

@@ -136,28 +136,28 @@ class TestErrorMessages(unittest.TestCase):
 
     def test_report_issue_message_contains_github(self):
         """Internal errors should suggest reporting on GitHub."""
-        messages = report_issue_error_message("SPAdes")
+        messages = report_issue_error_message("SPAdes", 0)
         combined = " ".join(messages)
         self.assertIn("github.com/ablab/spades", combined)
         self.assertIn("report", combined.lower())
 
     def test_report_issue_message_requests_files(self):
         """Internal errors should request log files for debugging."""
-        messages = report_issue_error_message("SPAdes")
+        messages = report_issue_error_message("SPAdes", 0)
         combined = " ".join(messages)
         self.assertIn("params.txt", combined)
         self.assertIn(".log", combined)
 
     def test_no_report_message_no_github(self):
         """User-end errors should NOT mention GitHub reporting."""
-        messages = no_report_error_message("SPAdes")
+        messages = no_report_error_message("SPAdes", 0)
         combined = " ".join(messages)
         self.assertNotIn("github", combined.lower())
         self.assertNotIn("report", combined.lower())
 
     def test_no_report_message_check_error(self):
         """User-end errors should tell user to check the error message."""
-        messages = no_report_error_message("SPAdes")
+        messages = no_report_error_message("SPAdes", 0)
         combined = " ".join(messages)
         self.assertIn("check the error message", combined.lower())
 

src/test/integration/test_error_codes.py

@@ -23,6 +23,8 @@
 """
 
 import os
+import shutil
+import signal
 import subprocess
 import sys
 import tempfile
@@ -33,29 +35,47 @@
 SPADES_SRC = os.path.dirname(os.path.dirname(SCRIPT_DIR))
 SPADES_ROOT = os.path.dirname(SPADES_SRC)
 
-# Try to find binaries in build directory or installed location
-BUILD_BIN = os.path.join(SPADES_ROOT, "build_spades", "bin")
-INSTALL_BIN = os.path.join(SPADES_ROOT, "bin")
-
-if os.path.exists(BUILD_BIN):
-    BIN_DIR = BUILD_BIN
-elif os.path.exists(INSTALL_BIN):
-    BIN_DIR = INSTALL_BIN
-else:
-    BIN_DIR = None
+# Try to find binaries: SPADES_BUILD_DIR env var takes priority, then common local paths
+_env_build_dir = os.environ.get("SPADES_BUILD_DIR")
+
+_candidates = []
+if _env_build_dir:
+    _candidates.append((_env_build_dir, os.path.join(SPADES_ROOT, "src/projects/")))
+_candidates += [
+    (os.path.join(SPADES_ROOT, "build_spades"), os.path.join(SPADES_ROOT, "src/projects/")),
+    (SPADES_ROOT, os.path.join(SPADES_ROOT, "share/")),
+]
+
+BIN_DIR = None
+SHARE_DIR = None
+for _build, _share in _candidates:
+    _bin = os.path.join(_build, "bin")
+    if os.path.exists(_bin):
+        BIN_DIR = _bin
+        SHARE_DIR = _share
+        break
+
+# Find spades.py using the same search order
+SPADES_PY = None
+for _build, _ in _candidates:
+    _candidate = os.path.join(_build, "bin", "spades.py")
+    if os.path.exists(_candidate):
+        SPADES_PY = _candidate
+        break
 
 # Test data directory
 TEST_DATA = os.path.join(SPADES_SRC, "test", "data")
 
 
-def run_command(cmd, timeout=30):
+def run_command(cmd, timeout=30, cwd=None):
     """Run a command and return (exit_code, stdout, stderr)."""
     try:
         result = subprocess.run(
             cmd,
             capture_output=True,
             text=True,
-            timeout=timeout
+            timeout=timeout,
+            cwd=cwd
         )
         return result.returncode, result.stdout, result.stderr
     except subprocess.TimeoutExpired:
@@ -152,14 +172,21 @@ def test_malformed_fastq(self):
 class TestInvalidParameterErrors(unittest.TestCase):
     """Test that invalid parameter errors return exit code 67."""
 
+    def setUp(self):
+        self.temp_dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.temp_dir, ignore_errors=True)
+
     def test_kmercount_invalid_kmer(self):
         """spades-kmercount with invalid k-mer should exit with error."""
-        # Using an even k-mer value which is invalid
+        # Using an even k-mer value which is invalid.
+        # Run in temp_dir so kmer_splitter_*/kmer_counter_* dirs are created there and cleaned up.
         cmd = [os.path.join(BIN_DIR, "spades-kmercount"),
                "-k", "32",  # Even k is invalid
-               "-o", "/tmp/test_out",
+               "-o", os.path.join(self.temp_dir, "test_out"),
                "/nonexistent/reads.fastq"]
-        exit_code, stdout, stderr = run_command(cmd)
+        exit_code, stdout, stderr = run_command(cmd, cwd=self.temp_dir)
         # Should fail with parameter error (67) or file not found (65)
         self.assertNotEqual(exit_code, 0,
                             f"Expected error, but command succeeded\nstdout: {stdout}")
@@ -295,9 +322,90 @@ def test_user_end_error_method(self):
             self.skipTest("Pipeline path not found")
 
 
+@unittest.skipIf(SPADES_PY is None, "spades.py not found")
+class TestSpadespyErrorCodes(unittest.TestCase):
+    """Tests that invoke spades.py directly and check exit codes and messages."""
+
+    def setUp(self):
+        self.tmp = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp, ignore_errors=True)
+
+    def _run(self, args, timeout=60):
+        """Run spades.py in its own process group so all children can be killed on timeout."""
+        cmd = [sys.executable, SPADES_PY] + args
+        try:
+            proc = subprocess.Popen(
+                cmd,
+                stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                text=True,
+                start_new_session=True  # new process group → kill whole tree on timeout
+            )
+            try:
+                stdout, stderr = proc.communicate(timeout=timeout)
+                return proc.returncode, stdout, stderr
+            except subprocess.TimeoutExpired:
+                os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
+                proc.wait()
+                return -1, "", "Command timed out"
+        except Exception as e:
+            return -1, "", str(e)
+
+    def test_nonexistent_input_file_exit_code(self):
+        """spades.py with non-existent -1 file should exit with 65 (InputFileNotFound)."""
+        exit_code, _, stderr = self._run(["-1", "/nonexistent/reads.fastq", "-o", self.tmp])
+        self.assertEqual(exit_code, 65,
+                         f"Expected 65 (InputFileNotFound), got {exit_code}\nstderr: {stderr}")
+
+    def test_nonexistent_input_file_no_report_message(self):
+        """File not found is a user-end error — should not suggest reporting a bug."""
+        _, stdout, stderr = self._run(["-1", "/nonexistent/reads.fastq", "-o", self.tmp])
+        self.assertFalse(has_report_bug_message(stdout + stderr),
+                         f"User-end error should not contain report bug message\nOutput: {stdout + stderr}")
+
+    def test_nonexistent_input_file_user_error_message(self):
+        """File not found should contain the 'check the error message' indicator."""
+        _, stdout, stderr = self._run(["-1", "/nonexistent/reads.fastq", "-o", self.tmp])
+        self.assertTrue(has_user_error_message(stdout + stderr),
+                        f"Expected user-end error message\nOutput: {stdout + stderr}")
+
+    def test_no_input_files_exit_code(self):
+        """spades.py with no input files should exit with 67 (InvalidParameter)."""
+        exit_code, _, stderr = self._run(["-o", self.tmp])
+        self.assertEqual(exit_code, 67,
+                         f"Expected 67 (InvalidParameter), got {exit_code}\nstderr: {stderr}")
+
+    def test_no_input_files_no_report_message(self):
+        """Missing input files is a user-end error — should not suggest reporting a bug."""
+        _, stdout, stderr = self._run(["-o", self.tmp])
+        self.assertFalse(has_report_bug_message(stdout + stderr),
+                         f"User-end error should not contain report bug message\nOutput: {stdout + stderr}")
+
+    @unittest.skipIf(BIN_DIR is None, "SPAdes binaries not found (needed for --test)")
+    def test_oom_exit_code(self):
+        """spades.py --test -m 1 should trigger OOM and exit with 68 (MemoryLimitExceeded)."""
+        exit_code, stdout, stderr = self._run(["-1", os.path.join(SHARE_DIR, "spades/test_dataset/ecoli_1K_1.fq.gz"),
+                                               "-2", os.path.join(SHARE_DIR, "spades/test_dataset/ecoli_1K_2.fq.gz"),
+                                               "-m", "1", "-o", self.tmp], timeout=120)
+        self.assertEqual(exit_code, 68,
+                         f"Expected 68 (MemoryLimitExceeded), got {exit_code}\nstdout: {stdout}\nstderr: {stderr}")
+
+    @unittest.skipIf(BIN_DIR is None, "SPAdes binaries not found (needed for --test)")
+    def test_oom_no_report_message(self):
+        """OOM (exit 68) is a user-end error — should not suggest reporting a bug."""
+        exit_code, stdout, stderr = self._run(["-1", os.path.join(SHARE_DIR, "spades/test_dataset/ecoli_1K_1.fq.gz"),
+                                               "-2", os.path.join(SHARE_DIR, "spades/test_dataset/ecoli_1K_2.fq.gz"),
+                                               "-m", "1", "-o", self.tmp], timeout=120)
+        if exit_code == 68:
+            self.assertFalse(has_report_bug_message(stdout + stderr),
+                             f"OOM error should not suggest reporting\nOutput: {stdout + stderr}")
+
+
 if __name__ == "__main__":
     # Print diagnostic info
     print(f"BIN_DIR: {BIN_DIR}")
+    print(f"SPADES_PY: {SPADES_PY}")
     print(f"TEST_DATA: {TEST_DATA}")
     if BIN_DIR:
         print(f"Binaries exist: {os.path.exists(BIN_DIR)}")