UnboundLocalError in graph.py when ModuleNotFoundError is caught but die_failure() doesn't exit

[immortal71]

Bug Description

The functions build_graph() and build_compare_report() in nettacker/core/graph.py have a bug that causes an UnboundLocalError when a ModuleNotFoundError is caught but the die_failure() function doesn't actually terminate the program (e.g., when die_failure is mocked in tests or if sys.exit() is intercepted).

Environment

• OS: Windows 11
• Python: 3.13.7 (though project supports 3.9-3.12)
• Nettacker Version: 0.4.0 (commit: 0cd1d0d)
• Repository: https://github.com/OWASP/Nettacker

Affected Code

File: nettacker/core/graph.py

Issue in build_graph() (lines 28-51):

def build_graph(graph_name, events):
    """
    build a graph
    ...
    """
    log.info(_("build_graph"))
    try:
        start = getattr(
            importlib.import_module(
                f"nettacker.lib.graph.{graph_name.rsplit('_graph')[0]}.engine"
            ),
            "start",
        )
    except ModuleNotFoundError:
        die_failure(_("graph_module_unavailable").format(graph_name))

    log.info(_("finish_build_graph"))
    return start(events)  # ← UnboundLocalError: 'start' referenced before assignment

Issue in build_compare_report() (lines 54-72):

def build_compare_report(compare_results):
    """
    build the compare report
    ...
    """
    log.info(_("build_compare_report"))
    try:
        build_report = getattr(
            importlib.import_module("nettacker.lib.compare_report.engine"),
            "build_report",
        )
    except ModuleNotFoundError:
        die_failure(_("graph_module_unavailable").format("compare_report"))

    log.info(_("finish_build_report"))
    return build_report(compare_results)  # ← UnboundLocalError: 'build_report' referenced before assignment

Root Cause

When ModuleNotFoundError is raised inside the try block:

1. The variable (start or build_report) is never assigned
2. The exception handler calls die_failure(), which normally calls sys.exit(1) to terminate the program
3. However, if sys.exit() doesn't actually exit (e.g., when mocked in tests), execution continues to the next line
4. The code tries to use the variable that was never assigned → UnboundLocalError

Evidence from Tests

The project's own test suite acknowledges this bug with @pytest.mark.xfail markers:

File: tests/core/test_graph.py (lines 35-39):

@patch("nettacker.core.graph.die_failure")
@patch("nettacker.core.graph.importlib.import_module", side_effect=ModuleNotFoundError)
@pytest.mark.xfail(reason="It will hit an UnboundLocalError")
def test_build_graph_module_not_found(mock_import_module, mock_die_failure):
    build_graph("missing_graph", [])
    mock_die_failure.assert_called_once()

File: tests/core/test_graph.py (lines 51-55):

@patch("nettacker.core.graph.die_failure")
@patch("nettacker.core.graph.importlib.import_module", side_effect=ModuleNotFoundError)
@pytest.mark.xfail(reason="It will hit an UnboundLocalError")
def test_build_compare_report_module_not_found(mock_import_module, mock_die_failure):
    build_compare_report({"some": "results"})
    mock_die_failure.assert_called_once()

These tests explicitly mark this as an expected failure with the exact error message: "It will hit an UnboundLocalError".

Expected Behavior

The function should either:

1. Properly exit via die_failure() without continuing execution, OR
2. Return immediately after calling die_failure() to prevent accessing unassigned variables

Actual Behavior

If die_failure() doesn't terminate the program (e.g., in test scenarios or if exception handling changes), the code raises:

UnboundLocalError: local variable 'start' referenced before assignment

or

UnboundLocalError: local variable 'build_report' referenced before assignment

Proposed Solution

Add explicit return statements after die_failure() calls to ensure the function doesn't continue execution:

def build_graph(graph_name, events):
    log.info(_("build_graph"))
    try:
        start = getattr(
            importlib.import_module(
                f"nettacker.lib.graph.{graph_name.rsplit('_graph')[0]}.engine"
            ),
            "start",
        )
    except ModuleNotFoundError:
        die_failure(_("graph_module_unavailable").format(graph_name))
        return  # ← Add this to prevent UnboundLocalError

    log.info(_("finish_build_graph"))
    return start(events)

The same fix applies to build_compare_report().

Impact

• Severity: Medium
• Test Coverage: Currently 2 tests are marked as xfail specifically because of this bug
• User Impact: In production, this likely doesn't manifest because sys.exit(1) successfully terminates. However, it:
  • Prevents proper unit testing with mocked die_failure
  • Could cause unexpected behavior if error handling or exit behavior changes
  • Makes the code fragile and harder to test

Steps to Reproduce

1. Mock die_failure() to prevent actual program exit
2. Mock importlib.import_module() to raise ModuleNotFoundError
3. Call build_graph() or build_compare_report()
4. Observe UnboundLocalError instead of graceful handling

If this issue is valid, I would like to work on a fix and open a PR.

[immortal71]

@arkid15r If this issue is valid, I would like to work on a fix and open a PR.

[joshua23maute]

build_graph()

except ModuleNotFoundError:
    die_failure(_("graph_module_unavailable").format(graph_name))
    return 

build_compare_report()

except ModuleNotFoundError:
    die_failure(_("graph_module_unavailable").format("compare_report"))
    return