Fix: Prevent Silent Exception Swallowing in Model Loading

[priyan17singh]

Fixes: #520

Summary

This PR fixes silent exception swallowing in model loading logic by:

• Replacing assert statements with explicit exceptions
• Removing broad except Exception: blocks
• Introducing specific exception handling for TorchScript fallback
• Preserving error context using exception chaining
• Improving optional dependency import handling

---

🚨 Problem

The current implementation:

• Uses assert for file validation (unsafe in production).
• Uses broad except Exception: blocks.
• Silently swallows critical errors (e.g., file not found, CUDA OOM, corrupted models).
• Provides misleading fallback messages.

This makes debugging difficult and hides real failures.

---

Changes Made

1. Safer File Validation

if not os.path.isfile(model):
    raise FileNotFoundError(f"Model file not found: {model}")

---

2. Improved Exception Handling

• Catch only expected exceptions during TorchScript loading
• Fallback to PyTorch loading only when appropriate
• Raise clear error if both loading methods fail

except (RuntimeError, EOFError) as jit_err:
    try:
        model = torch.load(model, map_location=self.device)
    except Exception as load_err:
        raise RuntimeError(
            f"Failed to load model. TorchScript error: {jit_err}, "
            f"PyTorch error: {load_err}"
        ) from load_err

---

3. Optional Dependency Handling

except ImportError:
    print("Optional dependency Open3D-ML3D not available")

---

🧪 Tests Added

Added tests/test_model_loading.py to validate error handling behavior.

✔️ Test Results

collected 4 items

test_detection_model_bad_file_raises_specific_error PASSED
test_segmentation_model_bad_file_raises_specific_error PASSED
test_detection_model_missing_file_raises_file_not_found PASSED
test_segmentation_model_missing_file_raises_file_not_found PASSED

4 passed, 1 warning in 12.76s

What is tested

• Invalid model file → raises appropriate exception
• Missing file → raises FileNotFoundError
• Ensures no silent fallback occurs on real errors

---

📊 Impact

• Improves debuggability
• Prevents silent failures
• Ensures correct error propagation
• Aligns with Python best practices
• Makes model loading more robust for production use

---

[dpascualhe]

Hi @priyan17singh , thanks for your contribution! Can you fix the conflicts? I'll review the PR after that

[priyan17singh]

Hi @dpascualhe, thank you for taking the time to review!

I've resolved the merge conflicts by rebasing my branch on top of the latest master and keeping the improved exception handling from this PR. The branch is now up to date — feel free to review when you get a chance!
Let me know if anything else needs to be addressed.

[dpascualhe]

Hi @priyan17singh ! Sorry for the delay, I'm afraid some conflicts have arised again, but the intent of your changes is looking good! Could you resolve the conflicts and let us know? I'll be vigilant to merge then.

[priyan17singh]

Hi @dpascualhe, thanks for the patience — conflicts are resolved now and the branch is up to date with master.

One thing worth flagging: tests/test_model_loading.py (added in this PR) requires torch, which isn't installed in the CI job — same as the rest of the project, torch is treated as an optional, user-installed dependency. I've guarded the import with pytest.importorskip("torch") so it skips cleanly in CI instead of breaking test collection for the whole suite (it was doing that before the fix). Locally, with torch installed, all 4 tests pass and verify the new exception-handling behavior (FileNotFoundError for missing files, RuntimeError with chained context for corrupted/invalid models).

Let me know if you'd rather I drop the test file entirely for now, or if the skip-guard approach works for you as is.