elixir-cloud-aai / cwl-WES

Trigger CWL workflows via GA4GH WES and TES
Apache License 2.0
16 stars 18 forks source link

Add CORS header for POST /runs #225

Closed manabuishii closed 3 years ago

manabuishii commented 3 years ago

Details

ADD CORS Header for POST /runs

Testing

Sorry no test code but tested on my console

Before fix

$ curl -X POST --dump-header - http://localhost:8080/ga4gh/wes/v1/runs
HTTP/1.1 400 BAD REQUEST
Server: gunicorn/19.9.0
Date: Tue, 09 Nov 2021 14:56:21 GMT
Connection: close
Content-Type: application/problem+json
Content-Length: 58

After

$ curl -X POST --dump-header - http://localhost:8080/ga4gh/wes/v1/runs
HTTP/1.1 404 NOT FOUND
Server: gunicorn/19.9.0
Date: Tue, 09 Nov 2021 15:00:02 GMT
Connection: close
Content-Type: application/problem+json
Content-Length: 205
Access-Control-Allow-Origin: *

{
  "detail": "The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.",
  "status": 404,
  "title": "Not Found",
  "type": "about:blank"
}

Documentation

GET method already have CORS header. but POST method do not have CORS header

Style

...

Closing issues

...

Credit

...

uniqueg commented 3 years ago

Thanks @manabuishii - looks good, but no cloue why the build fails. The logs don't show any errors. I'll just rerun and try again

inutano commented 3 years ago

Further testing showed that cwl-WES does send Access-Control-Allow-Origin: * header when it accepts a correct POST request. When a wrong request comes, it raises an error and returns a 400 bad request without the header. It's confusing, but I assume we need to fix the flask app to add the header even when the error was raised during the routing process, which may be tricky.

Manabu's comment shows that it returns 404 not found after the change was introduced, it should have caused another problem. I assume the test failure is somehow related to it.

I suggest to close this PR, but if one wants to avoid the confusing browser message, I won't stop :)