You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ep...@apache.org on 2022/10/18 13:10:57 UTC

[airflow] 41/41: Add separate error handler for 405(Method not allowed) errors (#26880)

This is an automated email from the ASF dual-hosted git repository.

ephraimanierobi pushed a commit to branch v2-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit 5bf28504902ff9eded740ff9b7f9da98ac404f5a
Author: Dakshin K <36...@users.noreply.github.com>
AuthorDate: Tue Oct 18 18:20:13 2022 +0530

    Add separate error handler for 405(Method not allowed) errors (#26880)
    
    Co-authored-by: Dakshin K <da...@gmail.com>
    (cherry picked from commit 8efb678e771c8b7e351220a1eb7eb246ae8ed97f)
---
 airflow/www/extensions/init_views.py               | 10 ++++-
 .../airflow/{not_found.html => error.html}         |  6 +--
 airflow/www/views.py                               | 19 ++++++++-
 tests/api_connexion/test_error_handling.py         | 47 ++++++++++++++++++----
 4 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/airflow/www/extensions/init_views.py b/airflow/www/extensions/init_views.py
index fcb2a87265..cad715085b 100644
--- 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 init_api_connexion(app: Flask) -> None:
         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 b/airflow/www/templates/airflow/error.html
similarity index 91%
rename from airflow/www/templates/airflow/not_found.html
rename to airflow/www/templates/airflow/error.html
index e9e1231e3a..cfb38a25b5 100644
--- a/airflow/www/templates/airflow/not_found.html
+++ b/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
index 8a45a15444..e27ad84c9f 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -482,15 +482,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
index 8793e7c150..86effad88c 100644
--- 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