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

Better handling of port bind failures #2818

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Better handling of port bind failures #2818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b9b54a2
Cleanup crash messages. Handle quiet exceptions quietly. Don't re-rai…
Tronic Sep 5, 2023
abdb5cb
Better diagnostic for permission denied on bind() for low ports.
Tronic Sep 5, 2023
ab19d7b
Cleanup.
Tronic Sep 5, 2023
e6c0f39
Exit with code 1 on quiet startup failures. Still delete unix socket …
Tronic Oct 20, 2023
69d493a
Quiet handling and better message on unix socket bind error too.
Tronic Oct 20, 2023
2e9965c
Merge branch 'main' into bind_exception
Tronic Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions sanic/mixins/startup.py
View file
Open in desktop
Expand Up @@ -49,7 +49,7 @@
from sanic.application.state import ApplicationServerInfo, Mode, ServerStage
from sanic.base.meta import SanicMeta
from sanic.compat import OS_IS_WINDOWS, StartMethod
from sanic.exceptions import ServerKilled
from sanic.exceptions import SanicException, ServerKilled
from sanic.helpers import Default, _default, is_atty
from sanic.http.constants import HTTP
from sanic.http.tls import get_ssl_context, process_to_context
Expand Down Expand Up @@ -1133,12 +1133,18 @@ def serve(
manager.run()
except ServerKilled:
exit_code = 1
except SanicException as e:
exit_code = 1
kwargs = primary_server_info.settings
if e.quiet:
error_logger.error(str(e))
else:
raise
except BaseException:
kwargs = primary_server_info.settings
error_logger.exception(
"Experienced exception while trying to serve"
)
raise
Tronic marked this conversation as resolved.
Show resolved Hide resolved
finally:
logger.info("Server Stopped")
for app in apps:
Expand Down
20 changes: 17 additions & 3 deletions sanic/server/socket.py
View file
Open in desktop
Expand Up @@ -47,10 +47,10 @@ def bind_unix_socket(path: str, *, mode=0o666, backlog=100) -> socket.socket:
path = os.path.abspath(path)
folder = os.path.dirname(path)
if not os.path.isdir(folder):
raise FileNotFoundError(f"Socket folder does not exist: {folder}")
raise FileNotFoundError("Socket folder does not exist")
try:
if not stat.S_ISSOCK(os.stat(path, follow_symlinks=False).st_mode):
raise FileExistsError(f"Existing file is not a socket: {path}")
raise FileExistsError("Existing file is not a socket")
except FileNotFoundError:
pass
# Create new socket with a random temporary name
Expand Down Expand Up @@ -103,7 +103,10 @@ def configure_socket(
unix = server_settings["unix"]
backlog = server_settings["backlog"]
if unix:
sock = bind_unix_socket(unix, backlog=backlog)
try:
sock = bind_unix_socket(unix, backlog=backlog)
except OSError as e:
raise ServerError(f"Error binding {unix}: {e}", quiet=True)
server_settings["unix"] = unix
if sock is None:
try:
Expand All @@ -112,6 +115,17 @@ def configure_socket(
server_settings["port"],
backlog=backlog,
)
except PermissionError:
p = server_settings["port"]
if not p or p >= 1024:
raise
addr = f"{server_settings['host']}:{p}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about ipv6 brackets here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should probably add a function in sanic/helpers for that and port numbers as these are getting repeated in many places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning to do that here, or should we make it a new issue to handle on its own?

error = ServerError(
f"Permission denied binding to {addr}.\n\n"
"Use `sudo sanic` to run on a privileged port.\n"
Tronic marked this conversation as resolved.
Show resolved Hide resolved
)
error.quiet = True
raise error
except OSError as e: # no cov
error = ServerError(
f"Sanic server could not start: {e}.\n\n"
Expand Down