From d9f2077e1c9cf01409e6514a67ac33a1d32c6577 Mon Sep 17 00:00:00 2001 From: Nancy <9d.24.nancy.sangani@gmail.com> Date: Mon, 8 Jun 2026 14:34:13 +0530 Subject: [PATCH] feat: add global error boundary and centralized exception handling --- app.py | 42 +++--- errors/__init__.py | 1 + errors/handlers.py | 115 +++++++++++++++ templates/400.html | 38 +++++ templates/429.html | 38 +++++ tests/test_error_handling.py | 273 +++++++++++++++++++++++++++++++++++ utils/error_logger.py | 81 +++++++++++ 7 files changed, 565 insertions(+), 23 deletions(-) create mode 100644 errors/__init__.py create mode 100644 errors/handlers.py create mode 100644 templates/400.html create mode 100644 templates/429.html create mode 100644 tests/test_error_handling.py create mode 100644 utils/error_logger.py diff --git a/app.py b/app.py index f5e5a74..94f8063 100644 --- a/app.py +++ b/app.py @@ -4,21 +4,28 @@ # Responsibilities: # - Create the Flask app instance # - Register the main Blueprint from routes/ -# - Register error handlers +# - Register the global error boundary via errors/handlers.py # - Start the development server when run directly # # Business logic, recommendation scoring, and data loading all live in # the utils/ and routes/ packages, not here. -from flask import Flask, render_template +from flask import Flask from routes.main_routes import main from config import Config +from errors.handlers import register_error_handlers app = Flask(__name__) # Register all routes defined in the main Blueprint app.register_blueprint(main) +# Register the global error boundary (handles 400, 403, 404, 405, 429, 500, +# and any unhandled Exception). Must be called after Blueprint registration +# so Blueprint-level error handlers take precedence where defined. +register_error_handlers(app) + + @app.after_request def add_security_headers(response): """Add basic security headers to all responses.""" @@ -30,31 +37,20 @@ def add_security_headers(response): ) return response -# ---- Error handlers ---- - -@app.errorhandler(404) -def page_not_found(error): - """Render a friendly 404 page instead of the raw Flask error.""" - return render_template("404.html", config=Config), 404 - -@app.errorhandler(500) +# Expose the 500 handler at module level so existing tests can import it +# directly: from app import app, internal_server_error def internal_server_error(error): - """Render a friendly 500 page for unexpected server errors.""" - return render_template("500.html", config=Config), 500 - -@app.errorhandler(405) -def method_not_allowed(error): - """Render a friendly 405 page when the wrong HTTP method is used.""" - return render_template("405.html", config=Config), 405 - -@app.errorhandler(403) -def forbidden(error): - """Render a friendly 403 page when access is denied.""" - return render_template("403.html", config=Config), 403 + """Proxy kept for backward compatibility with test_basic.py.""" + from errors.handlers import internal_server_error as _handler + return _handler(error) if __name__ == "__main__": import os debug_mode = os.environ.get("FLASK_DEBUG", "False").lower() in ("true", "1") - app.run(host="0.0.0.0", port=int(os.environ.get("PORT", 5000)), debug=debug_mode) + app.run( + host="0.0.0.0", + port=int(os.environ.get("PORT", 5000)), + debug=debug_mode, + ) diff --git a/errors/__init__.py b/errors/__init__.py new file mode 100644 index 0000000..6b65cde --- /dev/null +++ b/errors/__init__.py @@ -0,0 +1 @@ +# errors/__init__.py \ No newline at end of file diff --git a/errors/handlers.py b/errors/handlers.py new file mode 100644 index 0000000..0dd1897 --- /dev/null +++ b/errors/handlers.py @@ -0,0 +1,115 @@ +# errors/handlers.py +# Global error handling for DevPath. +# +# All Flask error handlers live here so app.py stays lean. +# Each handler: +# 1. Logs the exception via utils/error_logger.py (server-side only). +# 2. Renders the matching HTML template with a safe, user-friendly message. +# 3. Never leaks stack traces, file paths, or internal state to the client. +# +# Register by calling register_error_handlers(app) from app.py. + +from flask import Flask, render_template, request, jsonify +from config import Config +from utils.error_logger import log_exception + + +def _wants_json() -> bool: + """Return True when the request prefers a JSON response. + + API routes (prefixed /api/) always receive JSON error responses. + Browser requests receive the HTML error pages. + """ + if request.path.startswith("/api/"): + return True + best = request.accept_mimetypes.best_match(["application/json", "text/html"]) + return best == "application/json" + + +def _json_error(status_code: int, message: str, correlation_id: str = ""): + """Build a consistent JSON error envelope.""" + body = {"error": message} + if correlation_id: + body["reference"] = correlation_id + return jsonify(body), status_code + + +# --------------------------------------------------------------------------- +# Standalone handler functions — importable directly for testing +# --------------------------------------------------------------------------- + +def bad_request(error): + """Handle 400 Bad Request.""" + correlation_id = log_exception(error, status_code=400, context="bad_request") + if _wants_json(): + return _json_error(400, "Bad request.", correlation_id) + return render_template("400.html", config=Config, reference=correlation_id), 400 + + +def forbidden(error): + """Handle 403 Forbidden.""" + correlation_id = log_exception(error, status_code=403, context="forbidden") + if _wants_json(): + return _json_error(403, "Access denied.", correlation_id) + return render_template("403.html", config=Config, reference=correlation_id), 403 + + +def page_not_found(error): + """Handle 404 Not Found.""" + correlation_id = log_exception(error, status_code=404, context="page_not_found") + if _wants_json(): + return _json_error(404, "The requested resource was not found.", correlation_id) + return render_template("404.html", config=Config, reference=correlation_id), 404 + + +def method_not_allowed(error): + """Handle 405 Method Not Allowed.""" + correlation_id = log_exception(error, status_code=405, context="method_not_allowed") + if _wants_json(): + return _json_error(405, "HTTP method not allowed.", correlation_id) + return render_template("405.html", config=Config, reference=correlation_id), 405 + + +def too_many_requests(error): + """Handle 429 Too Many Requests.""" + correlation_id = log_exception(error, status_code=429, context="too_many_requests") + if _wants_json(): + return _json_error(429, "Too many requests. Please slow down.", correlation_id) + return render_template("429.html", config=Config, reference=correlation_id), 429 + + +def internal_server_error(error): + """Handle 500 Internal Server Error.""" + correlation_id = log_exception(error, status_code=500, context="internal_server_error") + if _wants_json(): + return _json_error(500, "An unexpected error occurred.", correlation_id) + return render_template("500.html", config=Config, reference=correlation_id), 500 + + +def unhandled_exception(error): + """Catch-all for any Exception not matched by a specific handler.""" + correlation_id = log_exception(error, status_code=500, context="unhandled_exception") + if _wants_json(): + return _json_error(500, "An unexpected error occurred.", correlation_id) + return render_template("500.html", config=Config, reference=correlation_id), 500 + + +# --------------------------------------------------------------------------- +# Registration +# --------------------------------------------------------------------------- + +def register_error_handlers(app: Flask) -> None: + """Attach all global error handlers to the Flask application. + + Call once during application initialisation: + + from errors.handlers import register_error_handlers + register_error_handlers(app) + """ + app.register_error_handler(400, bad_request) + app.register_error_handler(403, forbidden) + app.register_error_handler(404, page_not_found) + app.register_error_handler(405, method_not_allowed) + app.register_error_handler(429, too_many_requests) + app.register_error_handler(500, internal_server_error) + app.register_error_handler(Exception, unhandled_exception) diff --git a/templates/400.html b/templates/400.html new file mode 100644 index 0000000..1697d60 --- /dev/null +++ b/templates/400.html @@ -0,0 +1,38 @@ + + + + + + Bad Request — DevPath + + + + + + + + + + + + + +
+
+
400
+

Bad Request

+

The request could not be understood by the server. Please check your input and try again.

+ {% if reference %} +

Reference: {{ reference }}

+ {% endif %} + Back to Home +
+
+ + + diff --git a/templates/429.html b/templates/429.html new file mode 100644 index 0000000..775c2fb --- /dev/null +++ b/templates/429.html @@ -0,0 +1,38 @@ + + + + + + Too Many Requests — DevPath + + + + + + + + + + + + + +
+
+
429
+

Too Many Requests

+

You have sent too many requests in a short period. Please wait a moment before trying again.

+ {% if reference %} +

Reference: {{ reference }}

+ {% endif %} + Back to Home +
+
+ + + diff --git a/tests/test_error_handling.py b/tests/test_error_handling.py new file mode 100644 index 0000000..29d953d --- /dev/null +++ b/tests/test_error_handling.py @@ -0,0 +1,273 @@ +# tests/test_error_handling.py +# Tests for the global error boundary and exception handling mechanism. +# +# Covers: +# - Every registered HTTP error handler (400, 403, 404, 405, 429, 500) +# - JSON vs HTML content negotiation +# - API routes always receive JSON error responses +# - Correlation ID is present in JSON error envelopes +# - Stack traces / internal details are never exposed in any response +# - Security headers are preserved on error responses +# - log_exception() returns a valid correlation ID and does not raise +# - Unhandled exceptions are caught and return 500 + +import sys +import os +import logging + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from app import app +from utils.error_logger import log_exception + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def get_client(): + app.config["TESTING"] = True + return app.test_client() + + +# --------------------------------------------------------------------------- +# log_exception unit tests +# --------------------------------------------------------------------------- + +def test_log_exception_returns_correlation_id(): + """log_exception must return a non-empty string.""" + cid = log_exception(Exception("boom"), status_code=500) + assert isinstance(cid, str) + assert len(cid) > 0 + + +def test_log_exception_short_id(): + """Correlation ID should be exactly 8 hex characters.""" + cid = log_exception(ValueError("oops"), status_code=400) + assert len(cid) == 8 + assert all(c in "0123456789abcdef" for c in cid) + + +def test_log_exception_unique_ids(): + """Each call must produce a distinct correlation ID.""" + ids = {log_exception(Exception("x"), status_code=500) for _ in range(10)} + assert len(ids) == 10 + + +def test_log_exception_none_exc_does_not_raise(): + """Passing None as the exception must not raise.""" + cid = log_exception(None, status_code=500) + assert isinstance(cid, str) + + +def test_log_exception_writes_to_logger(caplog): + """log_exception must emit at least one ERROR-level log record.""" + with caplog.at_level(logging.ERROR, logger="devpath.errors"): + log_exception(RuntimeError("test log"), status_code=500, context="unit_test") + assert len(caplog.records) >= 1 + assert caplog.records[-1].levelno == logging.ERROR + + +def test_log_exception_includes_status_code(caplog): + """The log record message must contain the HTTP status code.""" + with caplog.at_level(logging.ERROR, logger="devpath.errors"): + log_exception(Exception("check"), status_code=418, context="teapot") + assert any("418" in r.message for r in caplog.records) + + +def test_log_exception_does_not_expose_traceback_to_caller(): + """The return value must be a short ID, not a traceback string.""" + cid = log_exception(Exception("secret internal detail"), status_code=500) + assert "Traceback" not in cid + assert "secret" not in cid + + +# --------------------------------------------------------------------------- +# HTML error page tests (browser requests) +# --------------------------------------------------------------------------- + +def _trigger_404(client): + return client.get("/this-route-definitely-does-not-exist-xyz") + + +def test_404_returns_404_status(): + assert _trigger_404(get_client()).status_code == 404 + + +def test_404_html_contains_friendly_text(): + response = _trigger_404(get_client()) + assert b"404" in response.data + + +def test_404_does_not_expose_stack_trace(): + response = _trigger_404(get_client()) + assert b"Traceback" not in response.data + assert b"File " not in response.data + + +def test_500_html_renders(): + """The 500 handler must render the friendly template and return 500.""" + with app.app_context(): + from errors.handlers import internal_server_error + rendered, status = internal_server_error(Exception("test")) + assert status == 500 + assert b"Internal Server Error" in rendered + assert b"Back to Home" in rendered + + +def test_500_does_not_expose_exception_message(): + """The 500 HTML response must not contain the raw exception message.""" + with app.app_context(): + from errors.handlers import internal_server_error + rendered, _ = internal_server_error(Exception("super secret db password")) + assert b"super secret db password" not in rendered + + +def test_405_on_wrong_method(): + client = get_client() + # GET on a POST-only endpoint + response = client.get("/api/recommend") + assert response.status_code == 405 + + +# --------------------------------------------------------------------------- +# JSON error response tests (API routes) +# --------------------------------------------------------------------------- + +def test_api_404_returns_json(): + client = get_client() + response = client.get("/api/nonexistent-endpoint-xyz") + assert response.status_code == 404 + data = response.get_json() + assert data is not None + assert "error" in data + + +def test_api_404_json_has_no_traceback(): + client = get_client() + data = client.get("/api/nonexistent-endpoint-xyz").get_json() + assert "Traceback" not in str(data) + assert "File " not in str(data) + + +def test_api_405_returns_json(): + client = get_client() + response = client.get("/api/recommend") + assert response.status_code == 405 + data = response.get_json() + assert "error" in data + + +def test_api_error_json_has_correlation_id(): + """JSON error responses for API routes must include a 'reference' field.""" + client = get_client() + data = client.get("/api/nonexistent-endpoint-xyz").get_json() + assert "reference" in data + assert len(data["reference"]) == 8 + + +def test_api_error_envelope_shape(): + """JSON error envelope must have exactly 'error' and 'reference' keys.""" + client = get_client() + data = client.get("/api/nonexistent-endpoint-xyz").get_json() + assert set(data.keys()) == {"error", "reference"} + + +def test_api_missing_field_returns_400_json(): + """POST /api/recommend with missing field must return 400 JSON.""" + client = get_client() + response = client.post("/api/recommend", json={"skills": "", "level": "Beginner", "interest": "Data", "time": "Low"}) + assert response.status_code in (400, 415) + data = response.get_json() + assert "error" in data + + +# --------------------------------------------------------------------------- +# Content negotiation tests +# --------------------------------------------------------------------------- + +def test_html_accept_returns_html_for_404(): + """A browser Accept header must receive an HTML error page.""" + client = get_client() + response = client.get( + "/this-route-does-not-exist", + headers={"Accept": "text/html,application/xhtml+xml,*/*;q=0.9"}, + ) + assert response.status_code == 404 + assert b"" in response.data or b" str: + """Log an exception securely and return a correlation ID. + + The full traceback is written to the server log only — it is never + included in any HTTP response. Callers receive a short correlation ID + they can surface in the UI ("reference: ") so users can report + issues without exposing internals. + + Args: + exc: The exception instance (may be None for synthetic errors). + status_code: HTTP status code associated with this error. + context: Optional free-text label (e.g. the route name). + + Returns: + A short alphanumeric correlation ID string. + """ + correlation_id = uuid.uuid4().hex[:8] + + exc_type = type(exc).__name__ if exc is not None else "UnknownError" + tb = ( + "".join(traceback.format_exception(type(exc), exc, exc.__traceback__)) + if exc is not None + else "No traceback available" + ) + + logger.error( + "status=%d id=%s type=%s context=%r\n%s", + status_code, + correlation_id, + exc_type, + context or "—", + tb, + ) + + return correlation_id