routes property returns None

[old-syniex]

Checklist

• [X ] The bug is reproducible against the latest release and/or master.
• [ X] There are no similar issues or pull requests to fix it yet.

Describe the bug

Elastic apm tries to collect information regarding the route by accessing routes property.
the implementations of BaseRoute such as Route, Mount says the return type is typing.List[BaseRoute].
In case that the app doesn't have "routes" it will return None.

Expected behavior

routes property will return empty list

Actual behavior

routes property will returns None

Debugging material

@property
    def routes(self) -> typing.List[BaseRoute]:
        return getattr(self.app, "routes", None)

Environment

• OS: Linux
• Python version: 3.9.4
• Starlette version: 0.14.2

[tomchristie]

I can't replicate the issue you're describing?...

>>> from starlette.applications import Starlette
>>> s = Starlette()
>>> s.routes
[]

[old-syniex]

I might didn't explained myself well.

>>> from starlette.applications import Starlette
>>> from starlette.staticfiles import StaticFiles
>>> app = Starlette()
>>> app.mount("/static",StaticFiles(directory="/"),name="static")
>>> for route in app.routes:
...     print(route)
...     print(route.routes)
... 
<starlette.routing.Mount object at 0x7f4a3b8ca340>
None
>>> 

[HassanAbouelela]

Using the reply above, I managed to replicate the behavior you described on the current master branch (d26f44b).

The behavior is in line with the following default value:

starlette/starlette/routing.py

Line 340 in b7aeae7

return getattr(self.app, "routes", None)

(Staticfiles does not define a routes property)

Given all that, a change to return an empty list would be fairly trivial. The main problem/question I see is that returning a None when no routes exist is fairly standard behavior in many parts of the codebase. None is the default value in the Mount class init and Starlette class init, amongst others. There are examples of the opposite though. Router class init uses an empty list when routes is None.

As far as I could tell, this behavior is not documented, so I'm not sure what the preferred behavior would be, but an argument could be made either way. Regardless, it may make sense to standardize this behavior for different API components, though I imagine this may be a breaking change.