dedent() fails if method has multi-line string

davidmcnabnz commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

If attempting to add a route for a handler whose code includes a multi-line string, the dedent() function crashes.

Code snippet

    async def handle_foo(self, request):
        return response.html(
            status=200,
            body="""<html>
 <head>
  <title>This is foo</title>
 </head>
 <body>
  <h1>Foo here</h1>
  <div>paragraph of foo</div>
 </body>
</html>
""")

Expected Behavior

I think either the dedent function should be able to cope with similar multi-line strings within a function, or the Sanic route add system should ideally not rely on being able to parse handler methods' source code.

Another argument supporting the latter is cases where someone may want to provide a Cython-compiled handler method.

How do you run Sanic?

As a script (app.run or Sanic.serve)

Operating System

Debian-family

Sanic Version

Sanic 23.3.0; Routing 22.8.0

Additional context

I'm attempting to run a dynamically-instantiated Sanic app within an existing event loop, so I can efficiently integrate Sanic with various other network clients and servers.

Tronic commented 1 year ago

Triage: Sanic analyzing/compiling the handler code fails because it uses dedent on the source code before AST parsing. Referring to @ahopkins

This is a bug that ought to be fixed. A possible alternative implementation would be to add if True: initial line after which any indentation will be valid to AST.

As a workaround, put your multi-line literals on top of your module (much better for how the code looks too), or use the html5tagger module that Sanic now ships with (since 23.3) and that will also create valid HTML with no extra whitespace in it (yours lacks DOCTYPE and has html/head/body tags that are not needed in HTML5).

https://github.com/sanic-org/html5tagger

Tronic commented 1 year ago

Using that module with Sanic wasn't documented yet but it looks like:

from sanic.response import html
from html5tagger import Document

async def handler(...):
    return html(
        Document("This is foo")
        .h1("Foo here")
        .p("Paragraph")
    )

ahopkins commented 1 year ago

I'm not sure I understand the issue. We don't touch the handlers usually. Only Sanic internals.

from sanic import Sanic, html

app = Sanic("TestApp")

@app.get("/")
async def handle_foo(_):
    return html(
        status=200,
        body="""<html>
 <head>
  <title>This is foo</title>
 </head>
 <body>
  <h1>Foo here</h1>
  <div>paragraph of foo</div>
 </body>
</html>
""",
    )

This runs just fine for me. Please LMK what I am missing, including the full traceback.

davidmcnabnz commented 1 year ago

I could also mention that I use the same in-method multi-line string pattern for SQL queries, eg (hypothetically):

Class MyHandler:
    ...
    async def handle_foo(self, req):
        query = """
SELECT
  *
FROM
  stuff JOIN otherstuff ON stuff.id=otherstuff.stuff_id
WHERE
  stuff.needs_doing=true
    AND stuff.task_id=$1
"""
   result = await self.fetch(query, req.whatever)
   return self.render(result)

In terms of hauling multi-line strings to top of module and assigning them to constants, I prefer to do that only when a string is needed by two or more methods.

Anyway, one hack-ish workaround for this bug could be scanning the source for the """ delimiter and refraining from dedenting lines following a line with """, up to and including a subsequent line containing a closing """

As for the HTML, I appreciate the suggestion. I worked around that issue by a habitual line-joiner pattern:

        raw = "\n".join([
            "<html>",
            " <head><title>foo</title></head>",
            " <body>",
            ...
            " </body>",
            "</html>",
           "",
           ]))

ahopkins commented 1 year ago

I'm still not sure I see the issue here. Happy for any more enlightenment.

sanic-org / sanic