TypeError in stop signal handler #3345

[mkf-sysangels]

TypeError in stop signal handler #3345

[TheophileDiot]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Component: src/ui

Fix for TypeError in SIGINT/SIGTERM signal handler

Added new temporary UI application (src/ui/temp.py) with corrected signal handling. The handle_stop() signal handler now invokes stop() with only the required positional argument (exit status code). Removed an extraneous second parameter (False) that was previously causing a TypeError during signal-driven shutdown.

Added inline comments documenting the correct stop() function signature and explaining why the second argument was spurious and removed.

• Lines changed: +5/−1 in signal handler logic
• Stability improvement: Eliminates crash on receipt of SIGINT/SIGTERM during shutdown
• Security impact: None
• User-visible changes: None
• Configuration/schema changes: None
• Documentation/tests: None updated

Note: A similar bug pattern exists in src/ui/app/utils.py (line 163: stop(0, False)) where the stop() function has a different signature that accepts the second parameter, so the bug is isolated to temp.py.

Walkthrough

The SIGINT/SIGTERM shutdown handler in src/ui/temp.py was corrected to invoke the stop() function with only its required positional argument (exit status), removing a spurious second argument that previously risked raising a TypeError during shutdown.

Changes

Cohort / File(s)	Summary
Shutdown Handler Fix src/ui/temp.py	Removed erroneous second argument (False) from stop() function call in the signal handler; the function signature expects only exit status. Added inline comments documenting the expected signature and correction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

A signal arrives at the shutdown door,
With arguments one too many before,
Out goes the spurious False with grace,
The handler now knows its proper place—
Clean exit, no TypeError's roar! 🛑

🚥 Pre-merge checks | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title references issue #3345 but does not follow the Conventional Commits or ' - description' format required. It reads more like an issue title than a commit message.	Reformat the title to follow either Conventional Commits (e.g., 'fix(ui): resolve TypeError in stop signal handler') or the project's ' - description' format (e.g., 'ui - fix TypeError in stop signal handler'). Remove the issue reference from the title; that belongs in the description.
Description check	❓ Inconclusive	The description is merely a duplicate of the title with no additional context, detail, or explanation of the changes made. It fails to provide meaningful information about the fix.	Expand the description with concrete details: explain that the stop() method expects only one positional argument (exit status), clarify what the spurious second argument (False) was, and describe the impact of the fix (prevents TypeError during shutdown).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ui/temp.py`:
- Around line 37-42: Replace the multi-line inline comments above handle_stop
with a concise single-line or short multi-line docstring inside the function
(e.g., describe that handle_stop handles signal shutdown and why only one
argument is accepted), leaving the LOGGER.info call intact; update the function
signature handle_stop(signum, frame) to include the docstring as the first
statement so the function is self-documented and conforms to the project's
docstring/style guidelines.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c9958a49-1353-4119-936a-0a119902c2b2

📥 Commits

Reviewing files that changed from the base of the PR and between a15aee6 and 8575497.

📒 Files selected for processing (1)

• src/ui/temp.py

📜 Review details

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Follow BunkerWeb's Python standards and security posture:

• Use snake_case for functions and variables, PascalCase for classes, and provide concise, accurate docstrings for public classes, functions, and methods.
• Respect Black formatting with a 160-character line limit and the existing pre-commit conventions. Do not insist on adding type annotations to previously untyped code, but accept them when added consistently.
• Catch specific exceptions; never use bare except:. Catching Exception is acceptable only at explicit process boundaries (for example scheduler loops, outer job runners, worker entrypoints, or graceful-shutdown boundaries) when the code logs enough context and either re-raises, returns an error status, or terminates safely.
• Never use os.system, subprocess.*(..., shell=True), eval, or exec. Pass subprocess arguments as a list and prefer explicit binary paths for privileged operations.
• Do not use unsafe deserialisers (pickle, marshal, shelve, jsonpickle, dill) for untrusted data. Use yaml.safe_load() rather than unsafe YAML loading.
• Open files with an explicit encoding (normally utf-8) and use with statements for files, sockets, database sessions, and temporary resources.
• Use secrets for token generation and hmac.compare_digest for token, HMAC, or signature comparisons.
• For HTTP clients (requests, httpx): always set an explicit timeout, validate destination URLs to block RFC1918/loopback/link-local ranges (SSRF), disable automatic redirects to internal hosts, and be careful with proxy settings.
• For filesystem operations: resolve paths with Path.resolve() and verify they remain under the intended base directory before reading or writing (path traversal).
• Use defusedxml rather than stdlib XML parsers for untrusted XML.
• For SQLAlchemy, use bound parameters and safe query construction. Never call text() with f-strings or .format(), and flag .execute() calls built...

Files:

• src/ui/temp.py

src/ui/**/*.py

⚙️ CodeRabbit configuration file

src/ui/**/*.py: src/ui/ is the admin UI and related backend:

• State-changing routes must enforce CSRF protection where browser sessions are used.
• Session, remember-me, and auth-cookie changes must preserve Secure, HttpOnly, and appropriate SameSite behaviour.
• Redirect targets such as next parameters must be validated against an allowlist or local-path policy.
• For templates and forms, escape user-controlled data, validate uploads by type and size, and keep files outside the web root unless there is a deliberate reviewed exception.
• BunkerWeb uses bcrypt; flag any move towards weak password hashing or plaintext credential handling.

Files:

• src/ui/temp.py

🔇 Additional comments (1)

src/ui/temp.py (1)

44-44: Correct arity fix in shutdown path

Line 44 now matches stop(status) and removes the runtime TypeError on SIGINT/SIGTERM handling. Good fix.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Replace inline rationale block with a function docstring

On Line 37-Line 42, prefer a concise docstring on handle_stop rather than multi-line inline comments; this keeps the function self-documented and aligned with project standards.

Proposed refactor

 def handle_stop(signum, frame):
-    # stop() accepts exactly one positional argument (exit status).
-    # Calling stop(0, False) raises TypeError at shutdown:
-    #   TypeError: stop() takes 1 positional argument but 2 were given
-    # The second argument was spurious and has been removed.
+    """Handle SIGINT/SIGTERM and terminate the temporary UI process cleanly."""
     LOGGER.info("Caught stop operation")
     LOGGER.info("Stopping web ui ...")
     stop(0)

As per coding guidelines, "**/*.py: Use snake_case for functions and variables, PascalCase for classes, and provide concise, accurate docstrings for public classes, functions, and methods."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def handle_stop(signum, frame):
	# stop() accepts exactly one positional argument (exit status).
	# Calling stop(0, False) raises TypeError at shutdown:
	# TypeError: stop() takes 1 positional argument but 2 were given
	# The second argument was spurious and has been removed.
	LOGGER.info("Caught stop operation")
	def handle_stop(signum, frame):
	"""Handle SIGINT/SIGTERM and terminate the temporary UI process cleanly."""
	LOGGER.info("Caught stop operation")
	LOGGER.info("Stopping web ui ...")
	stop(0)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/temp.py` around lines 37 - 42, Replace the multi-line inline comments
above handle_stop with a concise single-line or short multi-line docstring
inside the function (e.g., describe that handle_stop handles signal shutdown and
why only one argument is accepted), leaving the LOGGER.info call intact; update
the function signature handle_stop(signum, frame) to include the docstring as
the first statement so the function is self-documented and conforms to the
project's docstring/style guidelines.