Skip to content

Commit d539dcb

Browse files
committed
fix: guard set_scale against invalid concurrency_modifier returns (#458)
If a user-provided concurrency_modifier callback returns None, a non-integer, or a value < 1, the worker crashes with: TypeError: '<' not supported between instances of 'int' and 'NoneType' Validate the return value in set_scale(): reject None, non-int, and values < 1 (default to 1 with a warning). Wrap the callback in try/except so a raising modifier keeps the current concurrency instead of crashing the worker. Closes #458
1 parent 5f5831b commit d539dcb

2 files changed

Lines changed: 58 additions & 2 deletions

File tree

‎runpod/serverless/modules/rp_scale.py‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,23 @@ def __init__(self, config: Dict[str, Any]):
8585
self.stop_signals_fetcher_timeout = stop_signals_fetcher_timeout
8686

8787
async def set_scale(self):
88-
self.current_concurrency = self.concurrency_modifier(self.current_concurrency)
88+
try:
89+
result = self.concurrency_modifier(self.current_concurrency)
90+
except Exception as err:
91+
log.warning(
92+
"concurrency_modifier raised %s: %s — keeping concurrency at %d",
93+
type(err).__name__, err, self.current_concurrency,
94+
)
95+
result = self.current_concurrency
96+
97+
if not isinstance(result, int) or result < 1:
98+
log.warning(
99+
"concurrency_modifier returned invalid value %r — defaulting to 1",
100+
result,
101+
)
102+
result = 1
103+
104+
self.current_concurrency = result
89105

90106
if self.jobs_queue and (self.current_concurrency == self.jobs_queue.maxsize):
91107
# no need to resize

‎tests/test_serverless/test_modules/test_scale.py‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
import asyncio
12
import sys
23
import traceback
34
from unittest import TestCase
45
from unittest.mock import patch
56

6-
from runpod.serverless.modules.rp_scale import _handle_uncaught_exception
7+
from runpod.serverless.modules.rp_scale import JobScaler, _handle_uncaught_exception
78

89

910
class TestHandleUncaughtException(TestCase):
@@ -52,3 +53,42 @@ def test_handle_uncaught_exception_with_no_exception(self, mock_logger):
5253
def test_excepthook_not_set_when_start_not_invoked(self):
5354
assert sys.excepthook == sys.__excepthook__
5455
assert sys.excepthook != _handle_uncaught_exception
56+
57+
58+
class TestSetScaleConcurrencyValidation(TestCase):
59+
"""Tests that set_scale guards against invalid concurrency_modifier returns. (Fixes #458)"""
60+
61+
def _make_scaler(self, modifier):
62+
config = {"handler": lambda job: job, "concurrency_modifier": modifier}
63+
return JobScaler(config)
64+
65+
@patch("runpod.serverless.modules.rp_scale.log")
66+
def test_none_return_defaults_to_1(self, _mock_log):
67+
scaler = self._make_scaler(lambda _: None)
68+
asyncio.run(scaler.set_scale())
69+
assert scaler.current_concurrency == 1
70+
71+
@patch("runpod.serverless.modules.rp_scale.log")
72+
def test_negative_return_defaults_to_1(self, _mock_log):
73+
scaler = self._make_scaler(lambda _: -5)
74+
asyncio.run(scaler.set_scale())
75+
assert scaler.current_concurrency == 1
76+
77+
@patch("runpod.serverless.modules.rp_scale.log")
78+
def test_zero_return_defaults_to_1(self, _mock_log):
79+
scaler = self._make_scaler(lambda _: 0)
80+
asyncio.run(scaler.set_scale())
81+
assert scaler.current_concurrency == 1
82+
83+
@patch("runpod.serverless.modules.rp_scale.log")
84+
def test_exception_keeps_current(self, _mock_log):
85+
scaler = self._make_scaler(lambda _: (_ for _ in ()).throw(RuntimeError("boom")))
86+
scaler.current_concurrency = 4
87+
asyncio.run(scaler.set_scale())
88+
assert scaler.current_concurrency == 4
89+
90+
@patch("runpod.serverless.modules.rp_scale.log")
91+
def test_valid_int_applied(self, _mock_log):
92+
scaler = self._make_scaler(lambda _: 8)
93+
asyncio.run(scaler.set_scale())
94+
assert scaler.current_concurrency == 8

0 commit comments

Comments
 (0)