Skip to content

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

Merged
ephraimbuddy merged 1 commit intoapache:mainfrom
dakshin-k:405-error-response
Oct 18, 2022
Merged

Add separate error handler for 405(Method not allowed) errors#26880
ephraimbuddy merged 1 commit intoapache:mainfrom
dakshin-k:405-error-response

Conversation

@dakshin-k
Copy link
Contributor

@dakshin-k dakshin-k commented Oct 5, 2022

Previously, the webserver was using the same error handler for both 404 and 405 errors, and returning a 'Not Found' response for both cases. Hence the response status code was always 404, even when a 405(Method not allowed) was raised internally.

This PR creates a separate error handler for 405 errors, so it will return an appropriate response and status code.

closes: #26375

Results of manual testing

  • Returns a JSON response when hitting the REST APIs
dakshin@bluepenguin:~/projects/airflow$ curl -X PATCH -D - http://localhost:28080/api/v1/version
HTTP/1.1 405 METHOD NOT ALLOWED
Server: gunicorn
Date: Wed, 05 Oct 2022 08:35:30 GMT
Connection: close
Content-Type: application/problem+json
Content-Length: 142
X-Robots-Tag: noindex, nofollow
Set-Cookie: session=e2b057f0-12bb-4275-8d7c-ec10f86f291f.T49QxckJWF1FK5zE-nOHZMkMVg8; Expires=Fri, 04 Nov 2022 08:35:30 GMT; HttpOnly; Path=/; SameSite=Lax

{
  "detail": "The method is not allowed for the requested URL.",
  "status": 405,
  "title": "Method Not Allowed",
  "type": "about:blank"
}
  • Returns a HTML response for other URLs
dakshin@bluepenguin:~/projects/airflow$ curl -X PATCH -D - http://localhost:28080/home
HTTP/1.1 405 METHOD NOT ALLOWED
Server: Werkzeug/2.2.2 Python/3.7.14
Date: Wed, 05 Oct 2022 08:42:30 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 475
X-Robots-Tag: noindex, nofollow
Set-Cookie: session=1b653de3-8dfa-477f-a4b5-72bbef8bfaa9.vuiHP-jyaAoquKD_OAd5DEdQVmQ; Expires=Fri, 04 Nov 2022 08:42:30 GMT; HttpOnly; Path=/; SameSite=Lax
Connection: close



<!DOCTYPE html>
<html lang="en">
  <head>
    <title>Airflow 405</title>
    <link rel="icon" type="image/png" href="/static/pin_32.png">
  </head>
  <body>
    <div style="font-family: verdana; text-align: center; margin-top: 200px;">
      <img src="/static/pin_100.png" width="50px" alt="pin-logo" />
      <h1>Airflow 405</h1>
      <p>Received an invalid request.</p>
      <a href="/">Return to the main page</a>
      <p>bdb986291012</p>
    </div>
  </body>
</html>

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Oct 5, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 5, 2022

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@dakshin-k dakshin-k marked this pull request as ready for review October 5, 2022 11:01
uranusjr
uranusjr reviewed Oct 6, 2022
View reviewed changes
@dakshin-k dakshin-k force-pushed the 405-error-response branch 2 times, most recently from dd41868 to f2d1fc8 Compare October 8, 2022 08:37
@dakshin-k dakshin-k requested review from potiuk and uranusjr and removed request for ashb, bbovenzi, potiuk, ryanahamilton and uranusjr October 9, 2022 05:39
@potiuk
Copy link
Member

potiuk commented Oct 10, 2022

Static checks are failing - I suggest to install pre-commit locally (it will even fix things for you and running breeze static-checks --last-commit should run pre-commit on latest commit.

BTW. I will not be available for next two weeks (vacations) so I hope @uranusjr or someone else will merge it before :)

@dakshin-k
Copy link
Contributor Author

dakshin-k commented Oct 16, 2022

Fixed the static check issues, need approval to run the workflow again.

@uranusjr, do take a look if you can 😄

@eladkal eladkal added this to the Airflow 2.4.2 milestone Oct 17, 2022
@ephraimbuddy ephraimbuddy merged commit 8efb678 into apache:main Oct 18, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 18, 2022

Awesome work, congrats on your first merged pull request!

@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Oct 18, 2022
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
@dakshin-k @ephraimbuddy
Add separate error handler for 405(Method not allowed) errors (#26880)
5bf2850 
Co-authored-by: Dakshin K <dakshin.k1@gmail.com>
(cherry picked from commit 8efb678)
@dakshin-k dakshin-k deleted the 405-error-response branch October 18, 2022 21:48
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
@dakshin-k @ephraimbuddy
Add separate error handler for 405(Method not allowed) errors (#26880)
42176dc 
Co-authored-by: Dakshin K <dakshin.k1@gmail.com>
(cherry picked from commit 8efb678)
@ephraimbuddy ephraimbuddy mentioned this pull request Oct 20, 2022
Closed
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

@uranusjr uranusjr uranusjr approved these changes

@ephraimbuddy ephraimbuddy ephraimbuddy approved these changes

Assignees

No one assigned

Labels

area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes

Projects

None yet

Milestone

Airflow 2.4.2

Development

Successfully merging this pull request may close these issues.

Airflow Webserver returns incorrect HTTP Error Response for custom REST API endpoints

5 participants

@dakshin-k @potiuk @uranusjr @ephraimbuddy @eladkal