Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OPTIONS requests return a 500 error #1001

Closed
simonw opened this issue Oct 9, 2020 · 8 comments
Closed

OPTIONS requests return a 500 error #1001

simonw opened this issue Oct 9, 2020 · 8 comments
Labels
bug
Milestone
Datasette 0.50

Comments

@simonw
Copy link
Owner

simonw commented Oct 9, 2020 

% curl -vv -XOPTIONS https://latest.datasette.io/
*   Trying 216.58.195.83:443...
> OPTIONS / HTTP/1.1
> Host: latest.datasette.io
> User-Agent: curl/7.70.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 500 Internal Server Error
@simonw simonw added the bug label Oct 9, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

To get a traceback:

datasette . -p 8009 --pdb

And then:

curl -XOPTIONS http://127.0.0.1:8009

This causes the server to open a debugging prompt:

INFO:     127.0.0.1:59514 - "OPTIONS / HTTP/1.1" 500 Internal Server Error
> /Users/simon/Dropbox/Development/datasette/datasette/views/base.py(115)dispatch_request()
-> return await handler(request, *args, **kwargs)
(Pdb) list
110  	    def database_color(self, database):
111  	        return "ff0000"
112  	
113  	    async def dispatch_request(self, request, *args, **kwargs):
114  	        handler = getattr(self, request.method.lower(), None)
115  ->	        return await handler(request, *args, **kwargs)
116  	
117  	    async def render(self, templates, request, context=None):
118  	        context = context or {}
119  	        template = self.ds.jinja_env.select_template(templates)
120  	        template_context = {
(Pdb)

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

So the bug is in this code here:

datasette/datasette/views/base.py

Lines 113 to 115 in 703439b

async def dispatch_request(self, request, *args, **kwargs):
handler = getattr(self, request.method.lower(), None)
return await handler(request, *args, **kwargs)

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

What should an OPTIONS request return, anyway?

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020 

~ % curl -XOPTIONS https://www.google.com/
<!DOCTYPE html>
<html lang=en>
  <meta charset=utf-8>
  <meta name=viewport content="initial-scale=1, minimum-scale=1, width=device-width">
  <title>Error 405 (Method Not Allowed)!!1</title>

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

The www.djangoproject.com site returns an empty page:

~ % curl -vv -XOPTIONS https://www.djangoproject.com/
*   Trying 151.101.42.217:443...
* Connected to www.djangoproject.com (151.101.42.217) port 443 (#0)
* ALPN, offering http/1.1
* TLS 1.2 connection using TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
* Server certificate: osff2.map.fastly.net
* Server certificate: GlobalSign CloudSSL CA - SHA256 - G3
* Server certificate: GlobalSign Root CA
> OPTIONS / HTTP/1.1
> Host: www.djangoproject.com
> User-Agent: curl/7.70.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Connection: keep-alive
< Content-Length: 0
< Server: nginx
< Content-Type: text/html; charset=utf-8
< Allow: GET, HEAD, OPTIONS
< Content-Language: en
< X-Frame-Options: SAMEORIGIN
< X-Content-Type-Options: nosniff
< X-XSS-Protection: 1; mode=block
< Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
< Access-Control-Allow-Origin: https://code.djangoproject.com
< Accept-Ranges: bytes
< Date: Fri, 09 Oct 2020 00:59:42 GMT
< Via: 1.1 varnish
< X-Served-By: cache-sjc10047-SJC
< X-Cache: MISS
< X-Cache-Hits: 0
< X-Timer: S1602205182.833493,VS0,VE305
< Vary: Accept-Language, Accept-Encoding
< 
* Connection #0 to host www.djangoproject.com left intact
~ %

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/OPTIONS

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

I'm tempted to imitate Django Rest Framework here: https://github.com/encode/django-rest-framework/blob/2e721cdbc85a924d0b0f093b86fe1497b58fe287/rest_framework/views.py#L514-L521

    def options(self, request, *args, **kwargs):
        """
        Handler method for HTTP 'OPTIONS' request.
        """
        if self.metadata_class is None:
            return self.http_method_not_allowed(request, *args, **kwargs)
        data = self.metadata_class().determine_metadata(request, self)
        return Response(data, status=status.HTTP_200_OK)

That determine_metadata() default method does this: https://github.com/encode/django-rest-framework/blob/335054a5d36b352a58286b303b608b6bf48152f8/rest_framework/metadata.py#L29

Note the comment at the top:

    There are not any formalized standards for `OPTIONS` responses
    for us to base this on.

@simonw
Copy link
Owner Author

simonw commented Oct 9, 2020

I actually have a sensible OPTIONS implementation here:

datasette/datasette/views/base.py

Lines 154 to 165 in a648bb8

class DataView(BaseView):
name = ""
re_named_parameter = re.compile(":([a-zA-Z0-9_]+)")
def __init__(self, datasette):
self.ds = datasette
def options(self, request, *args, **kwargs):
r = Response.text("ok")
if self.ds.cors:
r.headers["Access-Control-Allow-Origin"] = "*"
return r

I'm going to set the default one to return a 405 (Method Not Allowed) like Google does.

@simonw simonw closed this as completed in 7249ac5 Oct 9, 2020
@simonw simonw added this to the Datasette 0.50 milestone Oct 9, 2020
simonw added a commit that referenced this issue Oct 9, 2020
@simonw
Support OPTIONS without 500, closes #1001
5d73cc0
simonw added a commit that referenced this issue Oct 9, 2020
@simonw
datasette.client internal requests mechanism
8f97b9b 
Closes #943

* Datasette now requires httpx>=0.15
* Support OPTIONS without 500, closes #1001
* Added internals tests for datasette.client methods
* Datasette's own test mechanism now uses httpx to simulate requests
* Tests simulate HTTP 1.1 now
* Added base_url in a bunch more places
* Mark some tests as xfail - will remove that when new httpx release ships: #1005
simonw added a commit that referenced this issue Oct 9, 2020
@simonw
Release 0.50
a61f0e4 
Refs #1001, #514, #891, #943, #969, #970, #978, #980, #996, #997

Closes #1002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
Datasette 0.50
Development

No branches or pull requests
1 participant
@simonw