Skip to content

Commit

Permalink
fix: Graceful error handling for port already in use (#5554)
Browse files Browse the repository at this point in the history
* Added error handling for port already in use

* Add unit tests
  • Loading branch information
hnnasit authored Jul 20, 2023
1 parent 64d9278 commit f04b336
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 30 deletions.
3 changes: 3 additions & 0 deletions samcli/commands/local/cli_common/invoke_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from samcli.lib.utils.async_utils import AsyncContext
from samcli.lib.utils.packagetype import ZIP
from samcli.lib.utils.stream_writer import StreamWriter
from samcli.local.docker.exceptions import PortAlreadyInUse
from samcli.local.docker.lambda_image import LambdaImage
from samcli.local.docker.manager import ContainerManager
from samcli.local.lambdafn.runtime import LambdaRuntime, WarmLambdaRuntime
Expand Down Expand Up @@ -317,6 +318,8 @@ def initialize_function_container(function: Function) -> None:
LOG.debug("Ctrl+C was pressed. Aborting containers initialization")
self._clean_running_containers_and_related_resources()
raise
except PortAlreadyInUse as port_inuse_ex:
raise port_inuse_ex
except Exception as ex:
LOG.error("Lambda functions containers initialization failed because of %s", ex)
self._clean_running_containers_and_related_resources()
Expand Down
3 changes: 2 additions & 1 deletion samcli/commands/local/invoke/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from samcli.commands.local.lib.exceptions import InvalidIntermediateImageError
from samcli.lib.telemetry.metric import track_command
from samcli.lib.utils.version_checker import check_newer_version
from samcli.local.docker.exceptions import ContainerNotStartableException
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -218,6 +218,7 @@ def do_cli( # pylint: disable=R0914
InvalidIntermediateImageError,
DebuggingNotSupported,
NoPrivilegeException,
PortAlreadyInUse,
) as ex:
raise UserException(str(ex), wrapped_from=ex.__class__.__name__) from ex
except DockerImagePullFailedException as ex:
Expand Down
3 changes: 2 additions & 1 deletion samcli/commands/local/start_api/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from samcli.commands.local.start_api.core.command import InvokeAPICommand
from samcli.lib.telemetry.metric import track_command
from samcli.lib.utils.version_checker import check_newer_version
from samcli.local.docker.exceptions import ContainerNotStartableException
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -243,6 +243,7 @@ def do_cli( # pylint: disable=R0914
InvalidLayerReference,
InvalidIntermediateImageError,
DebuggingNotSupported,
PortAlreadyInUse,
) as ex:
raise UserException(str(ex), wrapped_from=ex.__class__.__name__) from ex
except ContainerNotStartableException as ex:
Expand Down
3 changes: 2 additions & 1 deletion samcli/commands/local/start_lambda/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from samcli.commands.local.start_lambda.core.command import InvokeLambdaCommand
from samcli.lib.telemetry.metric import track_command
from samcli.lib.utils.version_checker import check_newer_version
from samcli.local.docker.exceptions import ContainerNotStartableException
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -223,6 +223,7 @@ def do_cli( # pylint: disable=R0914
InvalidLayerReference,
InvalidIntermediateImageError,
DebuggingNotSupported,
PortAlreadyInUse,
) as ex:
raise UserException(str(ex), wrapped_from=ex.__class__.__name__) from ex
except ContainerNotStartableException as ex:
Expand Down
14 changes: 9 additions & 5 deletions samcli/local/docker/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
from samcli.lib.utils.retry import retry
from samcli.lib.utils.tar import extract_tarfile
from samcli.local.docker.effective_user import ROOT_USER_ID, EffectiveUser

from .exceptions import ContainerNotStartableException
from .utils import NoFreePortsError, find_free_port, to_posix_path
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse
from samcli.local.docker.utils import NoFreePortsError, find_free_port, to_posix_path

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -310,8 +309,13 @@ def start(self, input_data=None):
# Get the underlying container instance from Docker API
real_container = self.docker_client.containers.get(self.id)

# Start the container
real_container.start()
try:
# Start the container
real_container.start()
except docker.errors.APIError as ex:
if "Ports are not available" in str(ex):
raise PortAlreadyInUse(ex.explanation.decode()) from ex
raise ex

@retry(exc=requests.exceptions.RequestException, exc_raise=ContainerResponseException)
def wait_for_http_response(self, name, event, stdout):
Expand Down
6 changes: 6 additions & 0 deletions samcli/local/docker/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ class NoFreePortsError(Exception):
"""
Exception to raise when there are no free ports found in a specified range.
"""


class PortAlreadyInUse(Exception):
"""
Exception to raise when the provided port is not available for use.
"""
24 changes: 14 additions & 10 deletions tests/unit/commands/local/invoke/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from unittest.mock import patch, Mock
from parameterized import parameterized, param

from samcli.local.docker.exceptions import ContainerNotStartableException
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse
from samcli.local.lambdafn.exceptions import FunctionNotFound
from samcli.lib.providers.exceptions import InvalidLayerReference
from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException
Expand Down Expand Up @@ -164,7 +164,7 @@ def test_cli_must_invoke_with_no_event(self, get_event_mock, InvokeContextMock):
@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
@patch("samcli.commands.local.invoke.cli._get_event")
def test_must_raise_user_exception_on_function_not_found(
self, side_effect_exception, expected_exectpion_message, get_event_mock, InvokeContextMock
self, side_effect_exception, expected_exception_message, get_event_mock, InvokeContextMock
):
event_data = "data"
get_event_mock.return_value = event_data
Expand All @@ -179,7 +179,7 @@ def test_must_raise_user_exception_on_function_not_found(
self.call_cli()

msg = str(ex_ctx.exception)
self.assertEqual(msg, expected_exectpion_message)
self.assertEqual(msg, expected_exception_message)

@parameterized.expand(
[
Expand All @@ -192,7 +192,7 @@ def test_must_raise_user_exception_on_function_not_found(
@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
@patch("samcli.commands.local.invoke.cli._get_event")
def test_must_raise_user_exception_on_function_local_invoke_image_not_found_for_IMAGE_packagetype(
self, side_effect_exception, expected_exectpion_message, get_event_mock, InvokeContextMock
self, side_effect_exception, expected_exception_message, get_event_mock, InvokeContextMock
):
event_data = "data"
get_event_mock.return_value = event_data
Expand All @@ -207,7 +207,7 @@ def test_must_raise_user_exception_on_function_local_invoke_image_not_found_for_
self.call_cli()

msg = str(ex_ctx.exception)
self.assertEqual(msg, expected_exectpion_message)
self.assertEqual(msg, expected_exception_message)

@parameterized.expand(
[
Expand All @@ -222,18 +222,18 @@ def test_must_raise_user_exception_on_function_local_invoke_image_not_found_for_
@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
@patch("samcli.commands.local.invoke.cli._get_event")
def test_must_raise_user_exception_on_invalid_sam_template(
self, exeception_to_raise, execption_message, get_event_mock, InvokeContextMock
self, exception_to_raise, exception_message, get_event_mock, InvokeContextMock
):
event_data = "data"
get_event_mock.return_value = event_data

InvokeContextMock.side_effect = exeception_to_raise
InvokeContextMock.side_effect = exception_to_raise

with self.assertRaises(UserException) as ex_ctx:
self.call_cli()

msg = str(ex_ctx.exception)
self.assertEqual(msg, execption_message)
self.assertEqual(msg, exception_message)

@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
@patch("samcli.commands.local.invoke.cli._get_event")
Expand All @@ -255,12 +255,16 @@ def test_must_raise_user_exception_on_invalid_env_vars(self, get_event_mock, Inv
ContainerNotStartableException("Container cannot be started, no free ports on host"),
"Container cannot be started, no free ports on host",
),
param(
PortAlreadyInUse("Container cannot be started, provided port already in use"),
"Container cannot be started, provided port already in use",
),
]
)
@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
@patch("samcli.commands.local.invoke.cli._get_event")
def test_must_raise_user_exception_on_function_no_free_ports(
self, side_effect_exception, expected_exectpion_message, get_event_mock, InvokeContextMock
self, side_effect_exception, expected_exception_message, get_event_mock, InvokeContextMock
):
event_data = "data"
get_event_mock.return_value = event_data
Expand All @@ -275,7 +279,7 @@ def test_must_raise_user_exception_on_function_no_free_ports(
self.call_cli()

msg = str(ex_ctx.exception)
self.assertEqual(msg, expected_exectpion_message)
self.assertEqual(msg, expected_exception_message)


class TestGetEvent(TestCase):
Expand Down
24 changes: 19 additions & 5 deletions tests/unit/commands/local/start_api/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
from unittest import TestCase
from unittest.mock import patch, Mock

from parameterized import parameterized
from parameterized import parameterized, param

from samcli.commands.local.start_api.cli import do_cli as start_api_cli
from samcli.commands.local.lib.exceptions import NoApisDefined, InvalidIntermediateImageError
from samcli.lib.providers.exceptions import InvalidLayerReference
from samcli.commands.exceptions import UserException
from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException
from samcli.commands.local.lib.exceptions import OverridesNotWellDefinedError
from samcli.local.docker.exceptions import ContainerNotStartableException
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse
from samcli.local.docker.lambda_debug_settings import DebuggingNotSupported


Expand Down Expand Up @@ -151,16 +151,30 @@ def test_must_raise_user_exception_on_invalid_env_vars(self, invoke_context_mock
expected = "bad env vars"
self.assertEqual(msg, expected)

@parameterized.expand(
[
param(
ContainerNotStartableException("no free ports on host to bind with container"),
"no free ports on host to bind with container",
),
param(
PortAlreadyInUse("provided port already in use"),
"provided port already in use",
),
]
)
@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
def test_must_raise_user_exception_on_no_free_ports(self, invoke_context_mock):
invoke_context_mock.side_effect = ContainerNotStartableException("no free ports on host to bind with container")
def test_must_raise_user_exception_on_no_free_ports(
self, side_effect_exception, expected_exception_message, invoke_context_mock
):
invoke_context_mock.side_effect = side_effect_exception

with self.assertRaises(UserException) as context:
self.call_cli()

msg = str(context.exception)
expected = "no free ports on host to bind with container"
self.assertEqual(msg, expected)
self.assertEqual(msg, expected_exception_message)

@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
def test_must_raise_user_exception_on_invalid_imageuri(self, invoke_context_mock):
Expand Down
25 changes: 19 additions & 6 deletions tests/unit/commands/local/start_lambda/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from unittest import TestCase
from unittest.mock import patch, Mock

from parameterized import parameterized
from parameterized import parameterized, param

from samcli.commands.local.start_lambda.cli import do_cli as start_lambda_cli
from samcli.lib.providers.exceptions import InvalidLayerReference
from samcli.commands.local.cli_common.user_exceptions import UserException
from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException
from samcli.local.docker.exceptions import ContainerNotStartableException
from samcli.local.docker.exceptions import ContainerNotStartableException, PortAlreadyInUse
from samcli.commands.local.lib.exceptions import OverridesNotWellDefinedError, InvalidIntermediateImageError
from samcli.local.docker.lambda_debug_settings import DebuggingNotSupported

Expand Down Expand Up @@ -133,16 +133,29 @@ def test_must_raise_user_exception_on_invalid_imageuri(self, invoke_context_mock
expected = "invalid imageuri"
self.assertEqual(msg, expected)

@parameterized.expand(
[
param(
ContainerNotStartableException("no free ports on host to bind with container"),
"no free ports on host to bind with container",
),
param(
PortAlreadyInUse("provided port already in use"),
"provided port already in use",
),
]
)
@patch("samcli.commands.local.cli_common.invoke_context.InvokeContext")
def test_must_raise_user_exception_on_no_free_ports(self, invoke_context_mock):
invoke_context_mock.side_effect = ContainerNotStartableException("no free ports on host to bind with container")
def test_must_raise_user_exception_on_no_free_ports(
self, side_effect_exception, expected_exception_message, invoke_context_mock
):
invoke_context_mock.side_effect = side_effect_exception

with self.assertRaises(UserException) as context:
self.call_cli()

msg = str(context.exception)
expected = "no free ports on host to bind with container"
self.assertEqual(msg, expected)
self.assertEqual(msg, expected_exception_message)

def call_cli(self):
start_lambda_cli(
Expand Down
27 changes: 26 additions & 1 deletion tests/unit/local/docker/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
from requests import RequestException

from samcli.lib.utils.packagetype import IMAGE
from samcli.local.docker.container import Container, ContainerResponseException, ContainerConnectionTimeoutException
from samcli.local.docker.container import (
Container,
ContainerResponseException,
ContainerConnectionTimeoutException,
PortAlreadyInUse,
)


class TestContainer_init(TestCase):
Expand Down Expand Up @@ -504,6 +509,26 @@ def test_must_not_start_if_container_is_not_created(self):
with self.assertRaises(RuntimeError):
self.container.start()

def test_docker_raises_port_inuse_error(self):
self.container.is_created.return_value = True

container_mock = Mock()
self.mock_docker_client.containers.get.return_value = container_mock
container_mock.start.side_effect = PortAlreadyInUse()

with self.assertRaises(PortAlreadyInUse):
self.container.start()

def test_docker_raises_api_error(self):
self.container.is_created.return_value = True

container_mock = Mock()
self.mock_docker_client.containers.get.return_value = container_mock
container_mock.start.side_effect = APIError("Mock Error")

with self.assertRaises(APIError):
self.container.start()

def test_must_not_support_input_data(self):
self.container.is_created.return_value = True

Expand Down

0 comments on commit f04b336

Please sign in to comment.