Skip to content

Commit 802d4f1

Browse files
chillaqchillaq
authored
Merge pull request #284 from splitio/invalid-apikey-timeout
Fire timeout exception with invalid apikey
2 parents 21b6eb8 + 1612c21 commit 802d4f1

File tree

12 files changed

+96
-98
lines changed

12 files changed

+96
-98
lines changed

‎CHANGES.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
9.2.1 (Nov 29, 2022)
2-
- Changed redis record type for impressions counts from list using rpush to hashed key using hincrby
1+
9.2.1 (Dec 1, 2022)
2+
- Changed redis record type for impressions counts from list using rpush to hashed key using hincrby.
3+
- Apply Timeout Exception when incorrect SDK API Key is used.
4+
- Changed potential initial fetching segment Warning to Debug in logging.
35

46
9.2.0 (Oct 14, 2022)
57
- Added a new impressions mode for the SDK called NONE , to be used in factory when there is no desire to capture impressions on an SDK factory to feed Split's analytics engine. Running NONE mode, the SDK will only capture unique keys evaluated for a particular feature flag instead of full blown impressions

‎splitio/client/factory.py‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,6 @@ def _build_in_memory_factory(api_key, cfg, sdk_url=None, events_url=None, # pyl
316316
'telemetry': TelemetryAPI(http_client, api_key, sdk_metadata),
317317
}
318318

319-
if not input_validator.validate_apikey_type(apis['segments']):
320-
return None
321-
322319
storages = {
323320
'splits': InMemorySplitStorage(),
324321
'segments': InMemorySegmentStorage(),
@@ -382,7 +379,7 @@ def _build_in_memory_factory(api_key, cfg, sdk_url=None, events_url=None, # pyl
382379
)
383380

384381
if preforked_initialization:
385-
synchronizer.sync_all()
382+
synchronizer.sync_all(max_retry_attempts=3)
386383
synchronizer._split_synchronizers._segment_sync.shutdown()
387384
return SplitFactory(api_key, storages, cfg['labelsEnabled'],
388385
recorder, manager, preforked_initialization=preforked_initialization)
@@ -394,7 +391,6 @@ def _build_in_memory_factory(api_key, cfg, sdk_url=None, events_url=None, # pyl
394391
return SplitFactory(api_key, storages, cfg['labelsEnabled'],
395392
recorder, manager, sdk_ready_flag)
396393

397-
398394
def _build_redis_factory(api_key, cfg):
399395
"""Build and return a split factory with redis-based storage."""
400396
sdk_metadata = util.get_metadata(cfg)

‎splitio/client/input_validator.py‎

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -446,32 +446,7 @@ def validate_attributes(attributes, method_name):
446446
class _ApiLogFilter(logging.Filter): # pylint: disable=too-few-public-methods
447447
def filter(self, record):
448448
return record.name not in ('SegmentsAPI', 'HttpClient')
449-
450-
451-
def validate_apikey_type(segment_api):
452-
"""
453-
Try to guess if the apikey is of browser type and let the user know.
454-
455-
:param segment_api: Segments API client.
456-
:type segment_api: splitio.api.segments.SegmentsAPI
457-
"""
458-
api_messages_filter = _ApiLogFilter()
459-
_logger = logging.getLogger('splitio.api.segments')
460-
try:
461-
_logger.addFilter(api_messages_filter) # pylint: disable=protected-access
462-
segment_api.fetch_segment('__SOME_INVALID_SEGMENT__', -1, FetchOptions())
463-
except APIException as exc:
464-
if exc.status_code == 403:
465-
_LOGGER.error('factory instantiation: you passed a browser type '
466-
+ 'api_key, please grab an api key from the Split '
467-
+ 'console that is of type sdk')
468-
return False
469-
finally:
470-
_logger.removeFilter(api_messages_filter) # pylint: disable=protected-access
471-
472-
# True doesn't mean that the APIKEY is right, only that it's not of type "browser"
473-
return True
474-
449+
475450

476451
def validate_factory_instantiation(apikey):
477452
"""

‎splitio/storage/inmemmory.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ def get(self, segment_name):
193193
with self._lock:
194194
fetched = self._segments.get(segment_name)
195195
if fetched is None:
196-
_LOGGER.warning(
196+
_LOGGER.debug(
197197
"Tried to retrieve nonexistant segment %s. Skipping",
198198
segment_name
199199
)

‎splitio/sync/manager.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
class Manager(object): # pylint:disable=too-many-instance-attributes
1616
"""Manager Class."""
1717

18+
_SYNC_ALL_ATTEMPTS = -1
1819
_CENTINEL_EVENT = object()
1920

2021
def __init__(self, ready_flag, synchronizer, auth_api, streaming_enabled, sdk_metadata, sse_url=None, client_key=None): # pylint:disable=too-many-arguments
@@ -59,9 +60,14 @@ def recreate(self):
5960
self._synchronizer._split_synchronizers._segment_sync.recreate()
6061

6162
def start(self):
62-
"""Start the SDK synchronization tasks."""
63+
"""
64+
Start the SDK synchronization tasks.
65+
66+
:param max_retry_attempts: apply max attempts if it set to absilute integer.
67+
:type max_retry_attempts: int
68+
"""
6369
try:
64-
self._synchronizer.sync_all()
70+
self._synchronizer.sync_all(self._SYNC_ALL_ATTEMPTS)
6571
self._ready_flag.set()
6672
self._synchronizer.start_periodic_data_recording()
6773
if self._streaming_enabled:

‎splitio/sync/synchronizer.py‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import abc
44
import logging
55
import threading
6+
import time
67

78
from splitio.api import APIException
9+
from splitio.util.backoff import Backoff
810

911

1012
_LOGGER = logging.getLogger(__name__)
@@ -212,6 +214,9 @@ def shutdown(self, blocking):
212214
class Synchronizer(BaseSynchronizer):
213215
"""Synchronizer."""
214216

217+
_ON_DEMAND_FETCH_BACKOFF_BASE = 10 # backoff base starting at 10 seconds
218+
_ON_DEMAND_FETCH_BACKOFF_MAX_WAIT = 30 # don't sleep for more than 1 minute
219+
215220
def __init__(self, split_synchronizers, split_tasks):
216221
"""
217222
Class constructor.
@@ -221,6 +226,9 @@ def __init__(self, split_synchronizers, split_tasks):
221226
:param split_tasks: tasks for starting/stopping tasks
222227
:type split_tasks: splitio.sync.synchronizer.SplitTasks
223228
"""
229+
self._backoff = Backoff(
230+
self._ON_DEMAND_FETCH_BACKOFF_BASE,
231+
self._ON_DEMAND_FETCH_BACKOFF_MAX_WAIT)
224232
self._split_synchronizers = split_synchronizers
225233
self._split_tasks = split_tasks
226234
self._periodic_data_recording_tasks = [
@@ -284,13 +292,20 @@ def synchronize_splits(self, till, sync_segments=True):
284292
_LOGGER.debug('Error: ', exc_info=True)
285293
return False
286294

287-
def sync_all(self):
288-
"""Synchronize all split data."""
289-
attempts = 3
290-
while attempts > 0:
295+
def sync_all(self, max_retry_attempts=-1):
296+
"""
297+
Synchronize all splits.
298+
299+
:param max_retry_attempts: apply max attempts if it set to absilute integer.
300+
:type max_retry_attempts: int
301+
"""
302+
retry_attempts = 0
303+
while True:
291304
try:
292305
if not self.synchronize_splits(None, False):
293-
attempts -= 1
306+
retry_attempts = self._retry_block(max_retry_attempts, retry_attempts)
307+
if max_retry_attempts != -1 and retry_attempts == -1:
308+
break
294309
continue
295310

296311
# Only retrying splits, since segments may trigger too many calls.
@@ -300,11 +315,23 @@ def sync_all(self):
300315
# All is good
301316
return
302317
except Exception as exc: # pylint:disable=broad-except
303-
attempts -= 1
304318
_LOGGER.error("Exception caught when trying to sync all data: %s", str(exc))
305319
_LOGGER.debug('Error: ', exc_info=True)
306-
307-
_LOGGER.error("Could not correctly synchronize splits and segments after 3 attempts.")
320+
retry_attempts = self._retry_block(max_retry_attempts, retry_attempts)
321+
if max_retry_attempts != -1 and retry_attempts == -1:
322+
break
323+
continue
324+
325+
_LOGGER.error("Could not correctly synchronize splits and segments after %d attempts.", retry_attempts)
326+
327+
def _retry_block(self, max_retry_attempts, retry_attempts):
328+
if max_retry_attempts != -1:
329+
retry_attempts += 1
330+
if retry_attempts > max_retry_attempts:
331+
return -1
332+
how_long = self._backoff.get()
333+
time.sleep(how_long)
334+
return retry_attempts
308335

309336
def shutdown(self, blocking):
310337
"""
@@ -468,8 +495,12 @@ def __init__(self, split_synchronizers, split_tasks):
468495
self._split_synchronizers = split_synchronizers
469496
self._split_tasks = split_tasks
470497

471-
def sync_all(self):
472-
"""Synchronize all split data."""
498+
def sync_all(self, max_retry_attempts=-1):
499+
"""
500+
Synchronize all splits.
501+
502+
:param max_retry_attempts: Not used, added for compatibility
503+
"""
473504
try:
474505
self._split_synchronizers.split_sync.synchronize_splits(None)
475506
except APIException as exc:

‎splitio/version.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = '9.2.1-rc'
1+
__version__ = '9.2.1-rc2'

‎tests/client/test_factory.py‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import os
66
import time
77
import threading
8+
import pytest
89
from splitio.client.factory import get_factory, SplitFactory, _INSTANTIATED_FACTORIES, Status,\
910
_LOGGER as _logger
1011
from splitio.client.config import DEFAULT_CONFIG
@@ -56,7 +57,10 @@ def _split_synchronizer(self, ready_flag, some, auth_api, streaming_enabled, sdk
5657
assert isinstance(factory._recorder._impression_storage, inmemmory.ImpressionStorage)
5758

5859
assert factory._labels_enabled is True
59-
factory.block_until_ready()
60+
try:
61+
factory.block_until_ready(1)
62+
except:
63+
pass
6064
assert factory.ready
6165
factory.destroy()
6266

@@ -129,12 +133,16 @@ def test_redis_client_creation(self, mocker):
129133
assert isinstance(factory._recorder._make_pipe(), RedisPipelineAdapter)
130134
assert isinstance(factory._recorder._event_sotrage, redis.RedisEventsStorage)
131135
assert isinstance(factory._recorder._impression_storage, redis.RedisImpressionsStorage)
132-
factory.block_until_ready()
136+
try:
137+
factory.block_until_ready(1)
138+
except:
139+
pass
133140
assert factory.ready
134141
factory.destroy()
135142

136143
def test_uwsgi_forked_client_creation(self):
137144
"""Test client with preforked initialization."""
145+
# pytest.set_trace()
138146
factory = get_factory('some_api_key', config={'preforkedInitialization': True})
139147
assert isinstance(factory._storages['splits'], inmemmory.InMemorySplitStorage)
140148
assert isinstance(factory._storages['segments'], inmemmory.InMemorySegmentStorage)
@@ -221,8 +229,11 @@ def _split_synchronizer(self, ready_flag, some, auth_api, streaming_enabled, sdk
221229

222230
# Start factory and make assertions
223231
factory = get_factory('some_api_key')
224-
factory.block_until_ready()
225-
assert factory.ready
232+
try:
233+
factory.block_until_ready(1)
234+
except:
235+
pass
236+
assert factory.ready is False
226237
assert factory.destroyed is False
227238

228239
factory.destroy()
@@ -304,8 +315,11 @@ def _split_synchronizer(self, ready_flag, some, auth_api, streaming_enabled, sdk
304315

305316
# Start factory and make assertions
306317
factory = get_factory('some_api_key')
307-
factory.block_until_ready()
308-
assert factory.ready
318+
try:
319+
factory.block_until_ready(1)
320+
except:
321+
pass
322+
assert factory.ready is False
309323
assert factory.destroyed is False
310324

311325
event = threading.Event()
@@ -470,7 +484,10 @@ def _get_storage_mock(self, name):
470484
'preforkedInitialization': True,
471485
}
472486
factory = get_factory("none", config=config)
473-
factory.block_until_ready(10)
487+
try:
488+
factory.block_until_ready(10)
489+
except:
490+
pass
474491
assert factory._status == Status.WAITING_FORK
475492
assert len(sync_all_mock.mock_calls) == 1
476493
assert len(start_mock.mock_calls) == 0
@@ -481,6 +498,7 @@ def _get_storage_mock(self, name):
481498

482499
assert clear_impressions._called == 1
483500
assert clear_events._called == 1
501+
factory.destroy()
484502

485503
def test_error_prefork(self, mocker):
486504
"""Test not handling fork."""
@@ -490,9 +508,12 @@ def test_error_prefork(self, mocker):
490508

491509
filename = os.path.join(os.path.dirname(__file__), '../integration/files', 'file2.yaml')
492510
factory = get_factory('localhost', config={'splitFile': filename})
493-
factory.block_until_ready(1)
494-
511+
try:
512+
factory.block_until_ready(1)
513+
except:
514+
pass
495515
_logger = mocker.Mock()
496516
mocker.patch('splitio.client.factory._LOGGER', new=_logger)
497517
factory.resume()
498518
assert _logger.warning.mock_calls == expected_msg
519+
factory.destroy()

‎tests/integration/test_client_e2e.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import json
44
import os
55
import threading
6+
import pytest
67

78
from redis import StrictRedis
89

‎tests/integration/test_streaming_e2e.py‎

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,6 @@ def test_happiness(self):
127127
'[?occupancy=metrics.publishers]control_sec'])
128128
assert qs['v'][0] == '1.1'
129129

130-
# Initial apikey validation
131-
req = split_backend_requests.get()
132-
assert req.method == 'GET'
133-
assert req.path == '/api/segmentChanges/__SOME_INVALID_SEGMENT__?since=-1'
134-
assert req.headers['authorization'] == 'Bearer some_apikey'
135-
136130
# Initial splits fetch
137131
req = split_backend_requests.get()
138132
assert req.method == 'GET'
@@ -334,12 +328,6 @@ def test_occupancy_flicker(self):
334328
'[?occupancy=metrics.publishers]control_sec'])
335329
assert qs['v'][0] == '1.1'
336330

337-
# Initial apikey validation
338-
req = split_backend_requests.get()
339-
assert req.method == 'GET'
340-
assert req.path == '/api/segmentChanges/__SOME_INVALID_SEGMENT__?since=-1'
341-
assert req.headers['authorization'] == 'Bearer some_apikey'
342-
343331
# Initial splits fetch
344332
req = split_backend_requests.get()
345333
assert req.method == 'GET'
@@ -514,12 +502,6 @@ def test_start_without_occupancy(self):
514502
'[?occupancy=metrics.publishers]control_sec'])
515503
assert qs['v'][0] == '1.1'
516504

517-
# Initial apikey validation
518-
req = split_backend_requests.get()
519-
assert req.method == 'GET'
520-
assert req.path == '/api/segmentChanges/__SOME_INVALID_SEGMENT__?since=-1'
521-
assert req.headers['authorization'] == 'Bearer some_apikey'
522-
523505
# Initial splits fetch
524506
req = split_backend_requests.get()
525507
assert req.method == 'GET'
@@ -704,12 +686,6 @@ def test_streaming_status_changes(self):
704686
'[?occupancy=metrics.publishers]control_sec'])
705687
assert qs['v'][0] == '1.1'
706688

707-
# Initial apikey validation
708-
req = split_backend_requests.get()
709-
assert req.method == 'GET'
710-
assert req.path == '/api/segmentChanges/__SOME_INVALID_SEGMENT__?since=-1'
711-
assert req.headers['authorization'] == 'Bearer some_apikey'
712-
713689
# Initial splits fetch
714690
req = split_backend_requests.get()
715691
assert req.method == 'GET'
@@ -931,12 +907,6 @@ def test_server_closes_connection(self):
931907
'[?occupancy=metrics.publishers]control_sec'])
932908
assert qs['v'][0] == '1.1'
933909

934-
# Initial apikey validation
935-
req = split_backend_requests.get()
936-
assert req.method == 'GET'
937-
assert req.path == '/api/segmentChanges/__SOME_INVALID_SEGMENT__?since=-1'
938-
assert req.headers['authorization'] == 'Bearer some_apikey'
939-
940910
# Initial splits fetch
941911
req = split_backend_requests.get()
942912
assert req.method == 'GET'
@@ -1168,12 +1138,6 @@ def test_ably_errors_handling(self):
11681138
'[?occupancy=metrics.publishers]control_sec'])
11691139
assert qs['v'][0] == '1.1'
11701140

1171-
# Initial apikey validation
1172-
req = split_backend_requests.get()
1173-
assert req.method == 'GET'
1174-
assert req.path == '/api/segmentChanges/__SOME_INVALID_SEGMENT__?since=-1'
1175-
assert req.headers['authorization'] == 'Bearer some_apikey'
1176-
11771141
# Initial splits fetch
11781142
req = split_backend_requests.get()
11791143
assert req.method == 'GET'

0 commit comments

Comments
 (0)