Add separate error handler for 405(Method not allowed) errors (#26880)

Co-authored-by: Dakshin K <dakshin.k1@gmail.com>
apache · Oct 18, 2022 · 8efb678 · 8efb678
1 parent ea55626
commit 8efb678
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 13 deletions.
diff --git a/airflow/www/extensions/init_views.py b/airflow/www/extensions/init_views.py
@@ -188,8 +188,7 @@ def init_api_connexion(app: Flask) -> None:
     from airflow.www import views
 
     @app.errorhandler(404)
-    @app.errorhandler(405)
-    def _handle_api_error(ex):
+    def _handle_api_not_found(ex):
         if request.path.startswith(base_path):
             # 404 errors are never handled on the blueprint level
             # unless raised from a view func so actual 404 errors,
@@ -199,6 +198,13 @@ def _handle_api_error(ex):
         else:
             return views.not_found(ex)
 
+    @app.errorhandler(405)
+    def _handle_method_not_allowed(ex):
+        if request.path.startswith(base_path):
+            return common_error_handler(ex)
+        else:
+            return views.method_not_allowed(ex)
+
     spec_dir = path.join(ROOT_APP_DIR, 'api_connexion', 'openapi')
     connexion_app = App(__name__, specification_dir=spec_dir, skip_error_handlers=True)
     connexion_app.app = app

diff --git a/airflow/www/templates/airflow/not_found.html → airflow/www/templates/airflow/error.html b/airflow/www/templates/airflow/not_found.html → airflow/www/templates/airflow/error.html
@@ -20,14 +20,14 @@
 <!DOCTYPE html>
 <html lang="en">
   <head>
-    <title>Airflow 404</title>
+    <title>Airflow {{ status_code }}</title>
     <link rel="icon" type="image/png" href="{{ url_for('static', filename='pin_32.png') }}">
   </head>
   <body>
     <div style="font-family: verdana; text-align: center; margin-top: 200px;">
       <img src="{{ url_for('static', filename='pin_100.png') }}" width="50px" alt="pin-logo" />
-      <h1>Airflow 404</h1>
-      <p>Page cannot be found.</p>
+      <h1>Airflow {{ status_code }}</h1>
+      <p>{{ error_message }}</p>
       <a href="/">Return to the main page</a>
       <p>{{ hostname }}</p>
     </div>

diff --git a/airflow/www/views.py b/airflow/www/views.py
@@ -489,15 +489,32 @@ def not_found(error):
     """Show Not Found on screen for any error in the Webserver"""
     return (
         render_template(
-            'airflow/not_found.html',
+            'airflow/error.html',
             hostname=get_hostname()
             if conf.getboolean('webserver', 'EXPOSE_HOSTNAME', fallback=True)
             else 'redact',
+            status_code=404,
+            error_message='Page cannot be found.',
         ),
         404,
     )
 
 
+def method_not_allowed(error):
+    """Show Method Not Allowed on screen for any error in the Webserver"""
+    return (
+        render_template(
+            'airflow/error.html',
+            hostname=get_hostname()
+            if conf.getboolean('webserver', 'EXPOSE_HOSTNAME', fallback=True)
+            else 'redact',
+            status_code=405,
+            error_message='Received an invalid request.',
+        ),
+        405,
+    )
+
+
 def show_traceback(error):
     """Show Traceback for a given error"""
     return (

diff --git a/tests/api_connexion/test_error_handling.py b/tests/api_connexion/test_error_handling.py
@@ -21,22 +21,55 @@ def test_incorrect_endpoint_should_return_json(minimal_app_for_api):
     client = minimal_app_for_api.test_client()
 
     # Given we have application with Connexion added
-    # When we hitting incorrect endpoint in API path
+    # When we are hitting incorrect endpoint in API path
 
-    resp_json = client.get("/api/v1/incorrect_endpoint").json
+    resp = client.get("/api/v1/incorrect_endpoint")
 
     # Then we have parsable JSON as output
 
-    assert 404 == resp_json["status"]
+    assert 'Not Found' == resp.json["title"]
+    assert 404 == resp.json["status"]
+    assert 404 == resp.status_code
+
+
+def test_incorrect_endpoint_should_return_html(minimal_app_for_api):
+    client = minimal_app_for_api.test_client()
 
     # When we are hitting non-api incorrect endpoint
 
-    resp_json = client.get("/incorrect_endpoint").json
+    resp = client.get("/incorrect_endpoint")
 
     # Then we do not have JSON as response, rather standard HTML
 
-    assert resp_json is None
+    assert resp.json is None
+    assert resp.mimetype == 'text/html'
+    assert resp.status_code == 404
+
+
+def test_incorrect_method_should_return_json(minimal_app_for_api):
+    client = minimal_app_for_api.test_client()
+
+    # Given we have application with Connexion added
+    # When we are hitting incorrect HTTP method in API path
+
+    resp = client.put("/api/v1/version")
 
-    resp_json = client.put("/api/v1/variables").json
+    # Then we have parsable JSON as output
+
+    assert 'Method Not Allowed' == resp.json["title"]
+    assert 405 == resp.json["status"]
+    assert 405 == resp.status_code
+
+
+def test_incorrect_method_should_return_html(minimal_app_for_api):
+    client = minimal_app_for_api.test_client()
+
+    # When we are hitting non-api incorrect HTTP method
+
+    resp = client.put("/")
+
+    # Then we do not have JSON as response, rather standard HTML
 
-    assert 'Method Not Allowed' == resp_json["title"]
+    assert resp.json is None
+    assert resp.mimetype == 'text/html'
+    assert resp.status_code == 405