Use string body instead of Json in compile request#217
Conversation
|
Does Scotty automatically handle OPTIONS HTTP requests if we specify a GET handler? If not, couldn't this have also been solved by adding a handler for that? options "/compile" $ do
Scotty.setHeader "Access-Control-Allow-Origin" "*"
Scotty.setHeader "Content-Type" "application/json" |
|
I doubt Scotty automatically handles OPTIONS, but I also don't think we should add a handler for it, because OPTIONS isn't necessary for Try PureScript, because we are only using GET and POST (which are CORS-safelisted methods: https://fetch.spec.whatwg.org/#methods). There were two problems here. Firstly, the request body was using the wrong format. It was using JSON-encoded string containing PureScript source code, rather than just PureScript source code. See #215 (comment). The only way of addressing this problem is by making the change in this PR. Secondly, the original problem was obscured because this error surfaced as a CORS error. I suspect this was because we are only returning the Access-Control-Allow-Origin header on success responses right now - we should include that header on all responses, including error responses. I imagine that this is still a problem. |
Isn't that for In my own experience, the dev tools network tab showed that an OPTIONS request was sent to scotty before the post request was sent. That's where the CORS issue occurred. When I added the above OPTIONS handler and reverted the change here, that issue no longer occurred and I instead got a 200. However, in my test the server code was also using the original |
|
I think it does still apply, yes - that's where the definition of CORS lives now, as I understand it. It appears that I was mistaken though; using a CORS-safelisted method is not sufficient to avoid OPTIONS requests. See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests. I suppose the use of a Content-Type of application/json as opposed to text/plain is what was triggering the OPTIONS request. In any case, I am quite confident that this is the correct fix. The server is not expecting JSON in the request body so we should not send JSON in the request body. |
|
Ok, thanks for clarifying. I'll drop this point. |
This fixes the CORS issue also noticed in #215, but we can merge this until folks have time to review that pull request.