Fix AttributeError when port is already in use

[dcabib]

Issue

Fixes #7244

Problem

When SAM CLI attempts to bind to a port that's already in use, users see a cryptic AttributeError: 'str' object has no attribute 'decode' instead of a helpful error message about which port is occupied.

This occurs because:

1. In docker-py >= 7.0, APIError.explanation is a str (not bytes)
2. The code calls .decode() on it, causing AttributeError
3. Case-sensitive check ("Ports") misses lowercase variations ("ports")

Solution

• Added safe_decode_docker_message() function that handles both bytes (docker-py < 7.0) and str (docker-py >= 7.0)
• Updated error detection to use case-insensitive check (.lower())
• Added 8 comprehensive unit tests covering all scenarios

Testing

• ✅ All 6931 existing tests pass
• ✅ Added 8 new tests (100% passing)
• ✅ Coverage maintained at 94.04%
• ✅ Lint and formatting checks pass
• ✅ Tested with samdev local invoke on occupied port

Result

Users now see the actual port error message:

Error: ports are not available: exposing port TCP 127.0.0.1:5005 -> 127.0.0.1:0: listen tcp4 127.0.0.1:5005: bind: address already in use

Instead of:

AttributeError: 'str' object has no attribute 'decode'

Checklist

• Code builds correctly
• New/updated tests pass
• All tests pass (make test)
• Coverage maintained at 94%+
• Lint checks pass (make lint)
• Formatting is correct (make black-check)
• Issue linked in commit and PR description
• Added comprehensive unit tests
• Backwards compatible (no breaking changes)

[vicheey]

Can we shorten the tests here with parameterization?

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 8f25f26..c3a280a
Files: 4
Comments: 1

[roger-zhangg]

LGTM — the fix is correct and minimal. The safe_decode_docker_message utility properly handles both bytes (docker-py < 7.0) and str (docker-py >= 7.0) return types, and the case-insensitive check is a good defensive improvement.

Two minor observations (non-blocking):

1. If ex.explanation is ever None (the APIError.__init__ signature allows explanation=None), the utility will return None. This is unlikely in practice since the "ports are not available" check on str(ex) implies the error is well-formed, but you could add a if message is None: return "" guard for extra safety.
2. Agree with the existing feedback from @vicheey — the three test cases for test_container.py could be consolidated with parameterization (e.g., @parameterized.expand) to reduce repetition.

Thanks for the contribution!

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 8f25f26..4c8af9b
Files: 4
Comments: 1

[aws-sam-tooling-bot]

[GENERAL] Re-raising unresolved feedback from @vicheey: the three new tests (test_docker_api_error_port_in_use_with_str_explanation, test_docker_api_error_port_in_use_with_bytes_explanation, test_docker_api_error_port_in_use_case_insensitive) share nearly identical setup and assertions, differing only in the explanation value passed to APIError and the expected substring. The file already imports parameterized at line 11, so consolidating these into a single parameterized test would reduce duplication and make it easier to add future cases (e.g. mixed-case, None, extra whitespace) without another copy-paste block.

Example:

@parameterized.expand([
   (
       "str_explanation_docker_py_ge_7",
       "ports are not available: listen tcp 127.0.0.1:5005: bind: address already in use",
       "ports are not available",
   ),
   (
       "bytes_explanation_docker_py_lt_7",
       b"ports are not available: listen tcp 127.0.0.1:5005: bind: address already in use",
       "ports are not available",
   ),
   (
       "case_insensitive_uppercase_ports",
       "Ports are not available: listen tcp 127.0.0.1:5005: bind: address already in use",
       "Ports are not available",
   ),
])
def test_docker_api_error_port_in_use(self, name, explanation, expectedsubstring):
   self.container.is_created.return_value = True

   container_mock = Mock()
   self.mock_docker_client.containers.get.return_value = container_mock

   message = explanation.decode() if isinstance(explanation, bytes) else explanation
   container_mock.start.side_effect = APIError(message, response=None, explanation=explanation)

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

   self.assertIn(expected_substring, str(context.exception))

This has not been explicitly dismissed by the author, so flagging again per review policy.

[roger-zhangg]

All review feedback addressed:

• None guard added to safe_decode_docker_message
• Tests parameterized per @vicheey's suggestion

Ready to merge.