Skip to content

Commit

Permalink
allow dict_merge to throw an error on rollbar.init + fix logging init
Browse files Browse the repository at this point in the history
- ``dict_merge`` is used in ``rollbar.init`` which potentially allows
  broken config to silently misconfigure the rollbar settings because it
  is not strictly copying the config passed in. This PR changes
  ``dict_merge`` to allow a strict mode so ``rollbar.init`` will throw
  an error instead of misconfiguring rollbar, and when ``dict_merge`` is
  used to send data to rollbar it uses the unstrict mode to not break at
  "runtime".

- When initializing rollbar via the ``RollbarHandler``, the logging
  config that is passed in is wrapped by a
  ``Converting{Dict,List,Tuple}`` which breaks ``copy.deepcopy``. This
  PR unwraps the converter container types to their base types so that
  the subsequent call to ``dict_merge`` does not break or hide this
  error.
  • Loading branch information
terencehonles committed Aug 14, 2020
1 parent 796d9c9 commit 57d9ccc
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 9 deletions.
8 changes: 4 additions & 4 deletions rollbar/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ def _report_exc_info(exc_info, request, extra_data, payload_data, level=None):
if not isinstance(extra_data, dict):
extra_data = {'value': extra_data}
if extra_trace_data:
extra_data = dict_merge(extra_data, extra_trace_data)
extra_data = dict_merge(extra_data, extra_trace_data, silence_errors=True)
data['custom'] = extra_data
if extra_trace_data and not extra_data:
data['custom'] = extra_trace_data
Expand All @@ -703,7 +703,7 @@ def _report_exc_info(exc_info, request, extra_data, payload_data, level=None):
data['server'] = _build_server_data()

if payload_data:
data = dict_merge(data, payload_data)
data = dict_merge(data, payload_data, silence_errors=True)

payload = _build_payload(data)
send_payload(payload, payload.get('access_token'))
Expand Down Expand Up @@ -783,7 +783,7 @@ def _report_message(message, level, request, extra_data, payload_data):
data['server'] = _build_server_data()

if payload_data:
data = dict_merge(data, payload_data)
data = dict_merge(data, payload_data, silence_errors=True)

payload = _build_payload(data)
send_payload(payload, payload.get('access_token'))
Expand Down Expand Up @@ -1014,7 +1014,7 @@ def _add_lambda_context_data(data):
}
}
if 'custom' in data:
data['custom'] = dict_merge(data['custom'], lambda_data)
data['custom'] = dict_merge(data['custom'], lambda_data, silence_errors=True)
else:
data['custom'] = lambda_data
except Exception as e:
Expand Down
7 changes: 5 additions & 2 deletions rollbar/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def is_builtin_type(obj):


# http://www.xormedia.com/recursively-merge-dictionaries-in-python.html
def dict_merge(a, b):
def dict_merge(a, b, silence_errors=False):
"""
Recursively merges dict's. not just simple a['key'] = b['key'], if
both a and bhave a key who's value is a dict then dict_merge is called
Expand All @@ -179,11 +179,14 @@ def dict_merge(a, b):
result = a
for k, v in b.items():
if k in result and isinstance(result[k], dict):
result[k] = dict_merge(result[k], v)
result[k] = dict_merge(result[k], v, silence_errors=silence_errors)
else:
try:
result[k] = copy.deepcopy(v)
except:
if not silence_errors:
raise six.reraise(*sys.exc_info())

result[k] = '<Uncopyable obj:(%s)>' % (v,)

return result
Expand Down
19 changes: 17 additions & 2 deletions rollbar/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import logging
import threading

from logging.config import ConvertingDict, ConvertingList, ConvertingTuple

import rollbar

# hack to fix backward compatibility in Python3
Expand All @@ -30,6 +32,17 @@
_checkLevel = lambda lvl: lvl


def resolve_logging_types(obj):
if isinstance(obj, (dict, ConvertingDict)):
return {k: resolve_logging_types(v) for k, v in obj.items()}
elif isinstance(obj, (list, ConvertingList)):
return [resolve_logging_types(i) for i in obj]
elif isinstance(obj, (tuple, ConvertingTuple)):
return tuple(resolve_logging_types(i) for i in obj)

return obj


class RollbarHandler(logging.Handler):
SUPPORTED_LEVELS = set(('debug', 'info', 'warning', 'error', 'critical'))

Expand All @@ -46,7 +59,8 @@ def __init__(self,
logging.Handler.__init__(self)

if access_token is not None:
rollbar.init(access_token, environment, **kw)
rollbar.init(
access_token, environment, **resolve_logging_types(kw))

self.notify_level = _checkLevel(level)

Expand Down Expand Up @@ -131,7 +145,8 @@ def emit(self, record):
}
}
}
payload_data = rollbar.dict_merge(payload_data, message_template)
payload_data = rollbar.dict_merge(
payload_data, message_template, silence_errors=True)

uuid = rollbar.report_exc_info(exc_info,
level=level,
Expand Down
2 changes: 1 addition & 1 deletion rollbar/test/test_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_dict_merge_dicts_select_poll(self):
p = poll()
a = {'a': {'b': 42}}
b = {'a': {'y': p}}
result = dict_merge(a, b)
result = dict_merge(a, b, silence_errors=True)

self.assertIn('a', result)
self.assertIn('b', result['a'])
Expand Down

0 comments on commit 57d9ccc

Please sign in to comment.