Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'master' into hamzawaleed01/upgrade-edx-enterprise-70f6f5a

Browse Source
This commit is contained in:
hamzawaleed01
2023-10-04 19:10:36 +05:00
committed by GitHub
parent 2255be06ad d9af2dfb36
commit 4887f7a615
10 changed files with 312 additions and 255 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
lms/envs/common.py
View File

@@ -2188,8 +2188,8 @@ MIDDLEWARE = [
# Before anything that looks at cookies, especially the session middleware
'openedx.core.djangoapps.cookie_metadata.middleware.CookieNameChange',
# Monitoring and logging for expected and ignored errors
'openedx.core.lib.request_utils.ExpectedErrorMiddleware',
# Monitoring and logging for ignored errors
'openedx.core.lib.request_utils.IgnoredErrorMiddleware',
'lms.djangoapps.mobile_api.middleware.AppVersionUpgrade',
'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware',
@@ -3330,7 +3330,7 @@ REST_FRAMEWORK = {
'DEFAULT_RENDERER_CLASSES': (
'rest_framework.renderers.JSONRenderer',
),
'EXCEPTION_HANDLER': 'openedx.core.lib.request_utils.expected_error_exception_handler',
'EXCEPTION_HANDLER': 'openedx.core.lib.request_utils.ignored_error_exception_handler',
'PAGE_SIZE': 10,
'URL_FORMAT_OVERRIDE': None,
'DEFAULT_THROTTLE_RATES': {

30
lms/static/js/learner_dashboard/spec/program_details_view_spec.js
View File

@@ -544,6 +544,28 @@ describe('Program Details View', () => {
.toContainText(body);
};
const testSubscriptionSunsetting = (state, heading, body) => {
const subscriptionData = {
...options.subscriptionData[0],
subscription_state: state,
};
// eslint-disable-next-line no-use-before-define
view = initView({
// eslint-disable-next-line no-undef
programData: $.extend({}, options.programData, {
subscription_eligible: false,
}),
isUserB2CSubscriptionsEnabled: true,
subscriptionData: [subscriptionData],
});
view.render();
expect(view.$('.upgrade-subscription')[0]).not.toBeInDOM();
expect(view.$('.upgrade-subscription .upgrade-button')).not
.toContainText(heading);
expect(view.$('.upgrade-subscription .subscription-info-brief')).not
.toContainText(body);
};
const initView = (updates) => {
// eslint-disable-next-line no-undef
const viewOptions = $.extend({}, options, updates);
@@ -709,8 +731,8 @@ describe('Program Details View', () => {
);
});
it('should render the get subscription link if program is subscription eligible', () => {
testSubscriptionState(
it('should not render the get subscription link if program is not active', () => {
testSubscriptionSunsetting(
'pre',
'Start 7-day free trial',
'$100/month USD subscription after trial ends. Cancel anytime.',
@@ -734,8 +756,8 @@ describe('Program Details View', () => {
);
});
it('should render appropriate subscription text when subscription is inactive', () => {
testSubscriptionState(
it('should not render appropriate subscription text when subscription is inactive', () => {
testSubscriptionSunsetting(
'inactive',
'Restart my subscription',
'$100/month USD subscription. Cancel anytime.',

6
lms/static/js/learner_dashboard/views/program_details_view.js
View File

@@ -215,9 +215,13 @@ class ProgramDetailsView extends Backbone.View {
&& this.remainingCourseCollection.length === 0
);
const isSubscriptionActiveSunsetting = (
this.subscriptionModel.get('subscriptionState') === 'active'
)
return (
this.options.isUserB2CSubscriptionsEnabled
&& this.options.programData.subscription_eligible
&& isSubscriptionActiveSunsetting
&& !programPurchasedWithoutSubscription
);
}

9
openedx/core/djangoapps/enrollments/views.py
View File

@@ -1066,7 +1066,9 @@ class EnrollmentAllowedView(APIView):
**Example Request**
POST /api/enrollment/v1/enrollment_allowed
POST /api/enrollment/v1/enrollment_allowed/
Note: The URL for this request must finish with /
Example request data:
```
@@ -1086,7 +1088,6 @@ class EnrollmentAllowedView(APIView):
- `auto_enroll` (optional, bool: default=false, _body_)
**Responses**
- 200: Success, enrollment allowed found.
- 400: Bad request, missing data.
- 403: Forbidden, you need to be staff.
- 409: Conflict, enrollment allowed already exists.
@@ -1122,7 +1123,9 @@ class EnrollmentAllowedView(APIView):
**Example Request**
DELETE /api/enrollment/v1/enrollment_allowed
DELETE /api/enrollment/v1/enrollment_allowed/
Note: The URL for this request must finish with /
Example request data:
```

15
openedx/core/lib/docs/decisions/0001-logging-and-monitoring-ignored-errors.rst
View File

@@ -4,7 +4,7 @@ Logging and monitoring ignored errors
Status
------
Accepted
Partially Superseded (see 0002-logging-and-monitoring-expected-errors-removed.rst)
Context
-------
@@ -14,7 +14,7 @@ We had two recent Production issues that took longer than necessary to diagnose
Decision
________
Add a capability for logging and monitoring ignored errors and expected errors. The feature will be configurable, the monitoring will be available by default, and the logging will be opt-in as needed.
Add a capability for logging and monitoring ignored errors. The feature will be configurable, the monitoring will be available by default, and the logging will be opt-in as needed.
Note: This feature is being added to edx-platform to start, but could be moved to edx-django-utils monitoring for use in other IDAs.
@@ -23,6 +23,13 @@ Consequence
The new capabilities have been built in edx-platform, although they could be moved to edx-django-utils monitoring in the future for use in other IDAs.
The new feature adds the ability to mark errors as expected, temporarily or permanently, even without "ignoring" them everywhere. For example, the errors and stacktraces would still appear, but it would be possible for alert conditions to ignore expected errors.
The new feature adds the ability to mark errors as ignored, temporarily or permanently, even without "ignoring" them everywhere. For example, the errors and stacktraces would still appear, but it would be possible for alert conditions to ignore errors.
See how_tos/logging-and-monitoring-expected-errors.rst for more information.
See how_tos/logging-and-monitoring-ignored-errors.rst for more information.
Changelog
---------
* **2023-09-28** - Changed status to "Partially Superseded", because expected errors will no longer be handled. See 0002-logging-and-monitoring-expected-errors-removed.rst for details.
* **2021-03-17** - Original "Accepted" ADR

51
openedx/core/lib/docs/decisions/0002-logging-and-monitoring-expected-errors-removed.rst Normal file
View File

@@ -0,0 +1,51 @@
Logging and monitoring of expected errors removed
=================================================
Status
------
Accepted
Context
-------
The setting ``EXPECTED_ERRORS`` was used for both expected and ignored errors in edx-platform.
The functionality supporting expected errors was added:
* Because New Relic didn't yet support this for Python, and
* Because it was thought that people may want more flexibility for marking errors as expected in addition to just the class name, and
* The setting provides a place to document why the change is being made.
Updates regarding these three original reasons:
* New Relic now supports expected errors, so the custom functionality is redundant and complicated, and
* This functionality has yet to be used, so it is also unnecessary.
Note that the need for custom functionality for ignored errors differs from expected errors in the following ways:
* New Relic still has problems from time to time where it stops ignoring errors that it is supposed to be ignoring, and this redundant functionality is used to catch that issue, and
* Since New Relic does not capture details of ignored errors, the custom functionality to log these errors can still come in handy if more details are needed.
This context can also be found in `[DEPR] Expected error part of EXPECTED_ERRORS`_.
.. _[DEPR] Expected error part of EXPECTED_ERRORS: https://github.com/openedx/edx-platform/issues/32405
Decision
________
The custom ignored error functionality proposed in 0001-logging-and-monitoring-ignored-errors.rst will remain in place, but the custom expected error functionality will be removed.
Consequence
-----------
* A number of code changes are required to remove the expected functionality.
* In many placed in the code, "expected" was used to mean "ignored and expected", and all such instances will be renamed to "ignored".
* The setting ``EXPECTED_ERRORS`` will be renamed to ``IGNORED_ERRORS``, which better matches how it was being used in the first place.
* The setting ``EXPECTED_ERRORS[REASON_EXPECTED]`` will be renamed to ``IGNORED_ERRORS[REASON_IGNORED]``.
* The setting toggle ``EXPECTED_ERRORS[IS_IGNORED]`` will be removed, because it will now always be True.
* The how-to will be renamed to how_tos/logging-and-monitoring-ignored-errors.rst.
* For more details, see https://github.com/openedx/edx-platform/pull/33184 where this work is underway.
* If anyone ever uses New Relic's expected error functionality, the reason for marking an error as expected would need to be captured elsewhere.

41
openedx/core/lib/docs/how_tos/logging-and-monitoring-expected-errors.rst
View File

@@ -1,41 +0,0 @@
How to log and monitor expected errors
======================================
What are expected and ignored errors?
-------------------------------------
Errors might be "expected" for various reasons. Some examples:
* Errors like ``PermissionDenied``, or ``Ratelimited``, where users or bots are prevented from doing something that is not allowed.
* Errors that we have already diagnosed, but may either not be worth fixing, or we are unable to fix for some time.
In some cases, a bug could introduce a large number errors where an "expected" error becomes an "unexpected" error. Or, there might be certain request paths for which the same error may be expected or unexpected.
A subset of "expected" errors are "ignored" errors. edX uses New Relic, which can be configured to ignore errors. These errors get suppressed and leave no (stack) trace, and won't trigger error alerts.
Monitoring expected errors
--------------------------
At a minimum, it is recommended that you add monitoring for any expected errors, including ignored errors. You do this by using the ``EXPECTED_ERRORS`` setting. For details on configuring, see the documentation for `EXPECTED_ERRORS settings and toggles on Readthedocs`_.
This will provide an ``error_expected`` custom attribute for every expected error. This custom attribute can be used in the following ways.
* Use ``WHERE error_expected IS NULL`` to filter out expected errors from most alert conditions.
* Set up special alerts for an overabundance of expected errors using ``WHERE error_expected IS NOT NULL``.
Additionally, a subset of expected errors that are configured as ignored will also get ``error_ignored_class`` and ``error_ignored_message`` custom attributes.
* Using New Relic terminology, this extra error class and message data will live on the Transaction and not the TransactionError, because ignored errors won't have a TransactionError.
* Use these additional custom attributes to help diagnose unexpected issues with ignored errors.
.. _EXPECTED_ERRORS settings and toggles on Readthedocs: https://edx.readthedocs.io/projects/edx-platform-technical/en/latest/search.html?q=EXPECTED_ERRORS&check_keywords=yes&area=default
Logging expected errors
-----------------------
Following the same documentation as for monitoring, you can also enable logging with or without a stack trace. This additional information may be useful for tracking down the source of a mysterious appearance of an otherwise expected error.
More targeted scoping of errors
-------------------------------
The initial implementation only enables this functionality by error ``module.Class``. If you need to scope the monitoring/logging for a more limited subset, like for certain requests, the expectation would be to enhance and document these new capabilities.

33
openedx/core/lib/docs/how_tos/logging-and-monitoring-ignored-errors.rst Normal file
View File

@@ -0,0 +1,33 @@
How to log and monitor ignored errors
======================================
What are ignored errors?
-------------------------------------
Operators of Open edX might use New Relic for monitoring, which can be configured to ignore errors. These errors get suppressed and leave no (stack) trace, and won't trigger error alerts.
Monitoring ignored errors
--------------------------
At a minimum, it is recommended that you add monitoring for any ignored errors. You do this by using the ``IGNORED_ERRORS`` setting. For details on configuring, see the documentation for `IGNORED_ERRORS settings and toggles on Readthedocs`_.
This will provide an ``error_ignored`` custom attribute for every ignored error. This custom attribute can be used in the following ways.
* Use ``SELECT * FROM TransactionError WHERE error_ignored IS True`` to find errors that should have been ignored, but are not. This may be due to a bug or misconfiguration in New Relic.
Additionally, a subset of ignored errors that are configured as ignored will also get ``error_ignored_class`` and ``error_ignored_message`` custom attributes.
* Using New Relic terminology, this extra error class and message data will live on the Transaction and not the TransactionError, because ignored errors won't have a TransactionError.
* Use these additional custom attributes to help diagnose unexpected issues with ignored errors.
.. _IGNORED_ERRORS settings and toggles on Readthedocs: https://edx.readthedocs.io/projects/edx-platform-technical/en/latest/search.html?q=IGNORED_ERRORS&check_keywords=yes&area=default
Logging ignored errors
-----------------------
Following the same documentation as for monitoring, you can also enable logging with or without a stack trace. This additional information may be useful for tracking down the source of a mysterious appearance of an otherwise ignored error.
More targeted scoping of errors
-------------------------------
The initial implementation only enables this functionality by error ``module.Class``. If you need to scope the monitoring/logging for a more limited subset, like for certain requests, the expectation would be to enhance and document these new capabilities.

191
openedx/core/lib/request_utils.py
View File

@@ -89,11 +89,11 @@ def course_id_from_url(url):
return None
def expected_error_exception_handler(exc, context):
def ignored_error_exception_handler(exc, context):
"""
Replacement for DRF's default exception handler to enable observing expected errors.
Replacement for DRF's default exception handler to enable observing ignored errors.
In addition to the default behaviour, add logging and monitoring for expected errors.
In addition to the default behaviour, add logging and monitoring for ignored errors.
"""
# Call REST framework's default exception handler first to get the standard error response.
response = exception_handler(exc, context)
@@ -103,13 +103,13 @@ def expected_error_exception_handler(exc, context):
except TypeError: # when context is not iterable
request = None
_log_and_monitor_expected_errors(request, exc, 'drf')
_log_and_monitor_ignored_errors(request, exc, 'drf')
return response
class ExpectedErrorMiddleware:
class IgnoredErrorMiddleware:
"""
Middleware to add logging and monitoring for expected errors.
Middleware to add logging and monitoring for ignored errors.
"""
def __init__(self, get_response):
self.get_response = get_response
@@ -120,57 +120,46 @@ class ExpectedErrorMiddleware:
def process_exception(self, request, exception):
"""
Add logging and monitoring of expected errors.
Add logging and monitoring of ignored errors.
"""
_log_and_monitor_expected_errors(request, exception, 'middleware')
_log_and_monitor_ignored_errors(request, exception, 'middleware')
# .. setting_name: EXPECTED_ERRORS
# .. setting_name: IGNORED_ERRORS
# .. setting_default: None
# .. setting_description: Used to configure logging and monitoring for expected errors.
# .. setting_description: Used to configure logging and monitoring for ignored errors.
# This setting is configured of a list of dicts. See setting and toggle annotations for
# ``EXPECTED_ERRORS[N]['XXX']`` for details of each item in the dict.
# If this setting is a non-empty list, all uncaught errors processed will get a ``checked_error_expected_from``
# attribute, whether they are expected or not. Those errors that are processed and match a 'MODULE_AND_CLASS'
# (documented elsewhere), will get an ``error_expected`` custom attribute. Unexpected errors would be errors with
# ``error_expected IS NULL``. For additional diagnostic information for ignored errors, see the
# EXPECTED_ERRORS[N]['IS_IGNORED'] annotation.
# ``IGNORED_ERRORS[N]['XXX']`` for details of each item in the dict.
# If this setting is a non-empty list, all uncaught errors processed will get a ``checked_error_ignored_from``
# attribute, whether they are ignored or not. See IGNORED_ERRORS[N]['MODULE_AND_CLASS'] annotation
# for details of monitoring added if the error is to be ignored.
# .. setting_warning: We use Django Middleware and a DRF custom error handler to find uncaught errors. Some errors may
# slip through the cracks, like ValidationError. Any error where ``checked_error_expected_from IS NULL`` is
# slip through the cracks, like ValidationError. Any error where ``checked_error_ignored_from IS NULL`` is
# an error that was not processed.
# .. setting_name: EXPECTED_ERRORS[N]['MODULE_AND_CLASS']
# .. setting_name: IGNORED_ERRORS[N]['MODULE_AND_CLASS']
# .. setting_default: None
# .. setting_description: Required error module and class name that is expected. For example,
# ``rest_framework.exceptions.PermissionDenied``.
# .. toggle_name: EXPECTED_ERRORS[N]['IS_IGNORED']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: True
# .. toggle_description: Set this to False if the errors are not ignored by monitoring, but only expected, like
# for temporary problems that may take some time to fix. If True, adds the custom attributes
# ``error_ignored_class`` and ``error_ignored_message`` to help diagnose issues with ignored errors, since
# this data is not otherwise available. For example of ignoring errors in New Relic, see:
# .. setting_description: Required error module and class name that is ignored. For example,
# ``rest_framework.exceptions.PermissionDenied``. If the current error matches the module and class
# defined here, the middleware will add the custom attributes ``error_ignored_class`` and ``error_ignored_message``
#. to help diagnose issues with ignored errors, since this data is not otherwise available.
# For an example of ignoring errors in New Relic, see:
# https://docs.newrelic.com/docs/agents/manage-apm-agents/agent-data/manage-errors-apm-collect-ignore-or-mark-expected/#ignore pylint: disable=line-too-long,useless-suppression
# To query for ignored errors, you would use ``error_ignored_class IS NOT NULL``.
# Note: This is defaulted to True because it will be easier for us to detect if True is not the correct value, by
# seeing that these errors aren't actually ignored.
# .. toggle_warning: At this time, this toggle does not actually configure the error to be ignored. It is meant to match
# the ignored error configuration found elsewhere. When monitoring, no errors should ever have the attribute
# .. setting_warning: At this time, an error that matches won't actually be ignored. These settings should be set to match
# the ignored error configuration found elsewhere, like in New Relic. When monitoring, no errors should ever have the attribute
# ``error_ignored_class``. Only Transactions should have this custom attribute. If found for an error, it means we
# are stating an error should be ignored when it is not actually configured as such, or the configuration is not
# are stating an error should be ignored when it is not actually configured as such, or the (New Relic) configuration is not
# working.
# .. toggle_use_cases: opt_out
# .. toggle_creation_date: 2021-03-11
# .. toggle_name: EXPECTED_ERRORS[N]['LOG_ERROR']
# .. toggle_name: IGNORED_ERRORS[N]['LOG_ERROR']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: If True, the error will be logged with a message like: "Expected error ...".
# .. toggle_description: If True, the error will be logged with a message like: "Ignored error ...".
# .. toggle_use_cases: opt_in
# .. toggle_creation_date: 2021-03-11
# .. toggle_name: EXPECTED_ERRORS[N]['LOG_STACK_TRACE']
# .. toggle_name: IGNORED_ERRORS[N]['LOG_STACK_TRACE']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: If True, the stacktrace will be included with the logging message.
@@ -178,123 +167,121 @@ class ExpectedErrorMiddleware:
# .. toggle_use_cases: opt_in
# .. toggle_creation_date: 2021-03-11
# .. setting_name: EXPECTED_ERRORS[N]['REASON_EXPECTED']
# .. setting_name: IGNORED_ERRORS[N]['REASON_IGNORED']
# .. setting_default: None
# .. setting_description: Required string explaining why the error is expected and/or ignored for documentation
# .. setting_description: Required string explaining why the error is ignored for documentation
# purposes.
# Warning: do not access this directly, but instead use _get_expected_error_settings_dict.
# EXPECTED ERRORS Django setting is processed and stored as a dict keyed by ERROR_MODULE_AND_CLASS.
_EXPECTED_ERROR_SETTINGS_DICT = None
# Warning: do not access this directly, but instead use _get_ignored_error_settings_dict.
# IGNORED_ERRORS Django setting is processed and stored as a dict keyed by ERROR_MODULE_AND_CLASS.
_IGNORED_ERROR_SETTINGS_DICT = None
def _get_expected_error_settings_dict():
def _get_ignored_error_settings_dict():
"""
Returns a dict of dicts of expected error settings used for logging and monitoring.
Returns a dict of dicts of ignored error settings used for logging and monitoring.
The contents of the EXPECTED_ERRORS Django Setting list is processed for efficient lookup by module.Class.
The contents of the IGNORED_ERRORS Django Setting list is processed for efficient lookup by module.Class.
Returns:
(dict): dict of dicts, mapping module-and-class name to settings for proper handling of expected errors.
Keys of the inner dicts use the lowercase version of the related Django Setting (e.g. 'REASON_EXPECTED' =>
'reason_expected').
(dict): dict of dicts, mapping module-and-class name to settings for proper handling of ignored errors.
Keys of the inner dicts use the lowercase version of the related Django Setting (e.g. 'REASON_IGNORED' =>
'reason_ignored').
Example return value::
{
'rest_framework.exceptions:PermissionDenied': {
'is_ignored': True,
'log_error': True,
'log_stack_trace': True,
'reason_expected': 'In most cases, signifies a user was trying to do something they cannot do. '
'reason_ignored': 'In most cases, signifies a user was trying to do something they cannot do. '
'However, an overabundance could indicate a bug, which could be monitored for.'
}
...
}
"""
global _EXPECTED_ERROR_SETTINGS_DICT
global _IGNORED_ERROR_SETTINGS_DICT
# Return cached processed mappings if already processed
if _EXPECTED_ERROR_SETTINGS_DICT is not None:
return _EXPECTED_ERROR_SETTINGS_DICT
if _IGNORED_ERROR_SETTINGS_DICT is not None:
return _IGNORED_ERROR_SETTINGS_DICT
expected_errors = getattr(settings, 'EXPECTED_ERRORS', None)
if expected_errors is None:
_EXPECTED_ERROR_SETTINGS_DICT = {}
return _EXPECTED_ERROR_SETTINGS_DICT
ignored_errors = getattr(settings, 'IGNORED_ERRORS', None)
if ignored_errors is None:
_IGNORED_ERROR_SETTINGS_DICT = {}
return _IGNORED_ERROR_SETTINGS_DICT
# Use temporary variable to build mappings to avoid multi-threading issue with a partially
# processed map. Worst case, it is processed more than once at start-up.
expected_error_settings_dict = {}
ignored_error_settings_dict = {}
try:
for index, expected_error in enumerate(expected_errors):
module_and_class = expected_error.get('MODULE_AND_CLASS')
processed_expected_error = {
'is_ignored': expected_error.get('IS_IGNORED', True),
'log_error': expected_error.get('LOG_ERROR', False),
'log_stack_trace': expected_error.get('LOG_STACK_TRACE', False),
'reason_expected': expected_error.get('REASON_EXPECTED'),
for index, ignored_error in enumerate(ignored_errors):
module_and_class = ignored_error.get('MODULE_AND_CLASS')
processed_ignored_error = {
'log_error': ignored_error.get('LOG_ERROR', False),
'log_stack_trace': ignored_error.get('LOG_STACK_TRACE', False),
'reason_ignored': ignored_error.get('REASON_IGNORED'),
}
# validate configuration
if not isinstance(module_and_class, str):
log.error(
"Skipping EXPECTED_ERRORS[%d] setting. 'MODULE_AND_CLASS' set to [%s] and should be module.Class, "
"Skipping IGNORED_ERRORS[%d] setting. 'MODULE_AND_CLASS' set to [%s] and should be module.Class, "
"like 'rest_framework.exceptions.PermissionDenied'.",
index, module_and_class
)
continue
if ':' in module_and_class:
log.warning(
"Replacing ':' with '.' in EXPECTED_ERRORS[%d]['MODULE_AND_CLASS'], which was set to %s. Note that "
"Replacing ':' with '.' in IGNORED_ERRORS[%d]['MODULE_AND_CLASS'], which was set to %s. Note that "
"monitoring and logging will not include the ':'.",
index, module_and_class
)
module_and_class = module_and_class.replace(":", ".")
if module_and_class in expected_error_settings_dict:
if module_and_class in ignored_error_settings_dict:
log.warning(
"EXPECTED_ERRORS[%d] setting is overriding an earlier setting. 'MODULE_AND_CLASS' [%s] is defined "
"IGNORED_ERRORS[%d] setting is overriding an earlier setting. 'MODULE_AND_CLASS' [%s] is defined "
"multiple times.",
index, module_and_class
)
if not processed_expected_error['reason_expected']:
if not processed_ignored_error['reason_ignored']:
log.error(
"Skipping EXPECTED_ERRORS[%d] setting. 'REASON_EXPECTED' is required to document why %s is an "
"expected error.",
"Skipping IGNORED_ERRORS[%d] setting. 'REASON_IGNORED' is required to document why %s is an "
"ignored error.",
index, module_and_class
)
continue
expected_error_settings_dict[module_and_class] = processed_expected_error
ignored_error_settings_dict[module_and_class] = processed_ignored_error
except Exception as e: # pylint: disable=broad-except
set_custom_attribute('expected_errors_setting_misconfigured', repr(e))
log.exception(f'Error processing setting EXPECTED_ERRORS. {repr(e)}')
set_custom_attribute('ignored_errors_setting_misconfigured', repr(e))
log.exception(f'Error processing setting IGNORED_ERRORS. {repr(e)}')
_EXPECTED_ERROR_SETTINGS_DICT = expected_error_settings_dict
return _EXPECTED_ERROR_SETTINGS_DICT
_IGNORED_ERROR_SETTINGS_DICT = ignored_error_settings_dict
return _IGNORED_ERROR_SETTINGS_DICT
def clear_cached_expected_error_settings():
def clear_cached_ignored_error_settings():
"""
Clears the cached expected error settings. Useful for testing.
Clears the cached ignored error settings. Useful for testing.
"""
global _EXPECTED_ERROR_SETTINGS_DICT
_EXPECTED_ERROR_SETTINGS_DICT = None
global _IGNORED_ERROR_SETTINGS_DICT
_IGNORED_ERROR_SETTINGS_DICT = None
def _log_and_monitor_expected_errors(request, exception, caller):
def _log_and_monitor_ignored_errors(request, exception, caller):
"""
Adds logging and monitoring for expected errors as needed.
Adds logging and monitoring for ignored errors as needed.
Arguments:
request: The request
exception: The exception
caller: Either 'middleware' or 'drf`
"""
expected_error_settings_dict = _get_expected_error_settings_dict()
if not expected_error_settings_dict:
ignored_error_settings_dict = _get_ignored_error_settings_dict()
if not ignored_error_settings_dict:
return
# 'module.Class', for example, 'django.core.exceptions.PermissionDenied'
@@ -303,8 +290,8 @@ def _log_and_monitor_expected_errors(request, exception, caller):
separator = '.' if exception_module else ''
module_and_class = f'{exception_module}{separator}{exception.__class__.__name__}'
# Set checked_error_expected_from custom attribute to potentially help find issues where errors are never processed.
set_custom_attribute('checked_error_expected_from', caller)
# Set checked_error_ignored_from custom attribute to potentially help find issues where errors are never processed.
set_custom_attribute('checked_error_ignored_from', caller)
# check if we already added logging/monitoring from a different caller
request_cache = RequestCache('openedx.core.lib.request_utils')
@@ -313,39 +300,37 @@ def _log_and_monitor_expected_errors(request, exception, caller):
cached_module_and_class = cached_handled_exception.value
# exception was already processed by a different caller
if cached_handled_exception.value == module_and_class:
set_custom_attribute('checked_error_expected_from', 'multiple')
set_custom_attribute('checked_error_ignored_from', 'multiple')
return
# We have confirmed using monitoring that it is very rare that middleware and drf handle different uncaught exceptions.
# We will leave this attribute in place, but it is not worth investing in a workaround, especially given that
# New Relic now offers its own expected error functionality, and this functionality may be simplified or removed.
# We will leave this attribute in place, but it is not worth investing in a workaround.
set_custom_attribute('unexpected_multiple_exceptions', cached_module_and_class)
log.warning(
"Unexpected scenario where different exceptions are handled by _log_and_monitor_expected_errors. "
"Unexpected scenario where different exceptions are handled by _log_and_monitor_ignored_errors. "
"See 'unexpected_multiple_exceptions' custom attribute. Skipping exception for %s.",
module_and_class,
)
return
request_cache.set('handled_exception', module_and_class)
if module_and_class not in expected_error_settings_dict:
if module_and_class not in ignored_error_settings_dict:
return
exception_message = str(exception)
set_custom_attribute('error_expected', True)
expected_error_settings = expected_error_settings_dict[module_and_class]
if expected_error_settings['is_ignored']:
# Additional error details are needed for ignored errors, because they are otherwise
# not available by our monitoring system, because they have been ignored.
set_custom_attribute('error_ignored_class', module_and_class)
set_custom_attribute('error_ignored_message', exception_message)
# Additional error details are needed for ignored errors, because they are otherwise
# not available by our monitoring system, because they have been ignored.
set_custom_attribute('error_ignored_class', module_and_class)
set_custom_attribute('error_ignored_message', exception_message)
if expected_error_settings['log_error']:
exc_info = exception if expected_error_settings['log_stack_trace'] else None
ignored_error_settings = ignored_error_settings_dict[module_and_class]
if ignored_error_settings['log_error']:
exc_info = exception if ignored_error_settings['log_stack_trace'] else None
request_path = getattr(request, 'path', 'request-path-unknown')
log.info(
'Expected error %s: %s: seen for path %s',
'Ignored error %s: %s: seen for path %s',
module_and_class,
exception_message,
request_path,

185
openedx/core/lib/tests/test_request_utils.py
View File

@@ -12,11 +12,11 @@ from django.test.utils import override_settings
from edx_django_utils.cache import RequestCache
from openedx.core.lib.request_utils import (
ExpectedErrorMiddleware,
_get_expected_error_settings_dict,
clear_cached_expected_error_settings,
IgnoredErrorMiddleware,
_get_ignored_error_settings_dict,
clear_cached_ignored_error_settings,
course_id_from_url,
expected_error_exception_handler,
ignored_error_exception_handler,
get_request_or_stub,
safe_get_host,
)
@@ -102,9 +102,9 @@ class RequestUtilTestCase(unittest.TestCase):
assert course_id.run == run
class TestGetExpectedErrorSettingsDict(unittest.TestCase):
class TestGetIgnoredErrorSettingsDict(unittest.TestCase):
"""
Tests for processing issues in _get_expected_error_settings_dict()
Tests for processing issues in _get_ignored_error_settings_dict()
Note: Although this is a private method, we have broken out the testing of
special cases so they don't have to be tested with the middleware and drf
@@ -112,100 +112,99 @@ class TestGetExpectedErrorSettingsDict(unittest.TestCase):
"""
def setUp(self):
super().setUp()
clear_cached_expected_error_settings()
clear_cached_ignored_error_settings()
def test_get_with_no_setting(self):
expected_error_settings_dict = _get_expected_error_settings_dict()
assert expected_error_settings_dict == {}
ignored_error_settings_dict = _get_ignored_error_settings_dict()
assert ignored_error_settings_dict == {}
@override_settings(EXPECTED_ERRORS=[])
@override_settings(IGNORED_ERRORS=[])
def test_get_with_empty_list_setting(self):
expected_error_settings_dict = _get_expected_error_settings_dict()
assert expected_error_settings_dict == {}
ignored_error_settings_dict = _get_ignored_error_settings_dict()
assert ignored_error_settings_dict == {}
@patch('openedx.core.lib.request_utils.log')
@override_settings(EXPECTED_ERRORS=[{}])
@override_settings(IGNORED_ERRORS=[{}])
def test_get_with_missing_module_and_class(self, mock_logger):
expected_error_settings_dict = _get_expected_error_settings_dict()
ignored_error_settings_dict = _get_ignored_error_settings_dict()
mock_logger.error.assert_called_once_with(
"Skipping EXPECTED_ERRORS[%d] setting. 'MODULE_AND_CLASS' set to [%s] and should be module.Class, like "
"Skipping IGNORED_ERRORS[%d] setting. 'MODULE_AND_CLASS' set to [%s] and should be module.Class, like "
"'rest_framework.exceptions.PermissionDenied'.",
0,
None,
)
assert expected_error_settings_dict == {}
assert ignored_error_settings_dict == {}
@patch('openedx.core.lib.request_utils.log')
@override_settings(EXPECTED_ERRORS=[
@override_settings(IGNORED_ERRORS=[
{
'MODULE_AND_CLASS': 'colon.separator.warning:Class',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}
])
def test_get_with_colon_in_class_and_module(self, mock_logger):
expected_error_settings_dict = _get_expected_error_settings_dict()
ignored_error_settings_dict = _get_ignored_error_settings_dict()
mock_logger.warning.assert_called_once_with(
"Replacing ':' with '.' in EXPECTED_ERRORS[%d]['MODULE_AND_CLASS'], which was set to %s. Note that "
"Replacing ':' with '.' in IGNORED_ERRORS[%d]['MODULE_AND_CLASS'], which was set to %s. Note that "
"monitoring and logging will not include the ':'.",
0,
'colon.separator.warning:Class',
)
assert 'colon.separator.warning.Class' in expected_error_settings_dict
assert 'colon.separator.warning.Class' in ignored_error_settings_dict
@patch('openedx.core.lib.request_utils.log')
@override_settings(EXPECTED_ERRORS=[
@override_settings(IGNORED_ERRORS=[
{
'MODULE_AND_CLASS': 'valid.module.DuplicateClass',
'REASON_EXPECTED': 'Because'
'REASON_IGNORED': 'Because'
},
{
'MODULE_AND_CLASS': 'valid.module.DuplicateClass',
'REASON_EXPECTED': 'Because overridden'
'REASON_IGNORED': 'Because overridden'
},
])
def test_get_with_duplicate_class_and_module(self, mock_logger):
expected_error_settings_dict = _get_expected_error_settings_dict()
ignored_error_settings_dict = _get_ignored_error_settings_dict()
mock_logger.warning.assert_called_once_with(
"EXPECTED_ERRORS[%d] setting is overriding an earlier setting. 'MODULE_AND_CLASS' [%s] is defined "
"IGNORED_ERRORS[%d] setting is overriding an earlier setting. 'MODULE_AND_CLASS' [%s] is defined "
"multiple times.",
1,
'valid.module.DuplicateClass',
)
assert 'valid.module.DuplicateClass' in expected_error_settings_dict
assert expected_error_settings_dict['valid.module.DuplicateClass']['reason_expected'] == 'Because overridden'
assert 'valid.module.DuplicateClass' in ignored_error_settings_dict
assert ignored_error_settings_dict['valid.module.DuplicateClass']['reason_ignored'] == 'Because overridden'
@patch('openedx.core.lib.request_utils.log')
@override_settings(EXPECTED_ERRORS=[{'MODULE_AND_CLASS': 'valid.module.and.class.ButMissingReason'}])
@override_settings(IGNORED_ERRORS=[{'MODULE_AND_CLASS': 'valid.module.and.class.ButMissingReason'}])
def test_get_with_missing_reason(self, mock_logger):
expected_error_settings_dict = _get_expected_error_settings_dict()
ignored_error_settings_dict = _get_ignored_error_settings_dict()
mock_logger.error.assert_called_once_with(
"Skipping EXPECTED_ERRORS[%d] setting. 'REASON_EXPECTED' is required to document why %s is an expected "
"Skipping IGNORED_ERRORS[%d] setting. 'REASON_IGNORED' is required to document why %s is an ignored "
"error.",
0, 'valid.module.and.class.ButMissingReason'
)
assert expected_error_settings_dict == {}
assert ignored_error_settings_dict == {}
@patch('openedx.core.lib.request_utils.log')
@override_settings(EXPECTED_ERRORS=['not-a-dict'])
@override_settings(IGNORED_ERRORS=['not-a-dict'])
def test_get_with_invalid_dict(self, mock_logger):
expected_error_settings_dict = _get_expected_error_settings_dict()
ignored_error_settings_dict = _get_ignored_error_settings_dict()
mock_logger.exception.assert_called_once_with(
'Error processing setting EXPECTED_ERRORS. AttributeError("\'str\' object has no attribute \'get\'")'
'Error processing setting IGNORED_ERRORS. AttributeError("\'str\' object has no attribute \'get\'")'
)
assert expected_error_settings_dict == {}
assert ignored_error_settings_dict == {}
@override_settings(EXPECTED_ERRORS=[{
@override_settings(IGNORED_ERRORS=[{
'MODULE_AND_CLASS': 'test.module.TestClass',
'REASON_EXPECTED': 'Because'
'REASON_IGNORED': 'Because'
}])
def test_get_with_defaults(self):
expected_error_settings_dict = _get_expected_error_settings_dict()
assert expected_error_settings_dict == {
ignored_error_settings_dict = _get_ignored_error_settings_dict()
assert ignored_error_settings_dict == {
'test.module.TestClass': {
'is_ignored': True,
'log_error': False,
'log_stack_trace': False,
'reason_expected': 'Because'
'reason_ignored': 'Because'
}
}
@@ -219,29 +218,29 @@ class CustomError2(Exception):
@ddt.ddt
class TestExpectedErrorMiddleware(unittest.TestCase):
class TestIgnoredErrorMiddleware(unittest.TestCase):
"""
Tests for ExpectedErrorMiddleware
Tests for IgnoredErrorMiddleware
"""
def setUp(self):
super().setUp()
RequestCache.clear_all_namespaces()
clear_cached_expected_error_settings()
clear_cached_ignored_error_settings()
self.mock_request = RequestFactory().get('/test')
self.mock_exception = CustomError1('Test failure')
def test_get_response(self):
expected_response = Mock()
middleware = ExpectedErrorMiddleware(lambda _: expected_response)
middleware = IgnoredErrorMiddleware(lambda _: expected_response)
response = middleware(self.mock_request)
assert response == expected_response
@patch('openedx.core.lib.request_utils.set_custom_attribute')
@patch('openedx.core.lib.request_utils.log')
def test_process_exception_no_expected_errors(self, mock_logger, mock_set_custom_attribute):
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
def test_process_exception_no_ignored_errors(self, mock_logger, mock_set_custom_attribute):
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
mock_logger.info.assert_not_called()
mock_set_custom_attribute.assert_not_called()
@@ -249,61 +248,60 @@ class TestExpectedErrorMiddleware(unittest.TestCase):
@patch('openedx.core.lib.request_utils.set_custom_attribute')
@patch('openedx.core.lib.request_utils.log')
@ddt.data(None, [])
def test_process_exception_with_empty_expected_errors(
self, expected_errors_setting, mock_logger, mock_set_custom_attribute,
def test_process_exception_with_empty_ignored_errors(
self, ignored_errors_setting, mock_logger, mock_set_custom_attribute,
):
with override_settings(EXPECTED_ERRORS=expected_errors_setting):
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
with override_settings(IGNORED_ERRORS=ignored_errors_setting):
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
mock_logger.info.assert_not_called()
mock_set_custom_attribute.assert_not_called()
@override_settings(EXPECTED_ERRORS=[{
@override_settings(IGNORED_ERRORS=[{
'MODULE_AND_CLASS': 'test.module.TestException',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}])
@patch('openedx.core.lib.request_utils.set_custom_attribute')
@patch('openedx.core.lib.request_utils.log')
def test_process_exception_not_matching_expected_errors(self, mock_logger, mock_set_custom_attribute):
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
def test_process_exception_not_matching_ignored_errors(self, mock_logger, mock_set_custom_attribute):
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
mock_logger.info.assert_not_called()
mock_set_custom_attribute.assert_called_once_with('checked_error_expected_from', 'middleware')
mock_set_custom_attribute.assert_called_once_with('checked_error_ignored_from', 'middleware')
@override_settings(EXPECTED_ERRORS=[
@override_settings(IGNORED_ERRORS=[
{
'MODULE_AND_CLASS': 'test.module.TestException',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
},
{
'MODULE_AND_CLASS': 'openedx.core.lib.tests.test_request_utils.CustomError1',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}
])
@patch('openedx.core.lib.request_utils.set_custom_attribute')
@patch('openedx.core.lib.request_utils.log')
def test_process_exception_expected_error_with_defaults(self, mock_logger, mock_set_custom_attribute):
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
def test_process_exception_ignored_error_with_defaults(self, mock_logger, mock_set_custom_attribute):
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
mock_logger.info.assert_not_called()
mock_set_custom_attribute.assert_has_calls(
[
call('checked_error_expected_from', 'middleware'),
call('error_expected', True),
call('checked_error_ignored_from', 'middleware'),
call('error_ignored_class', 'openedx.core.lib.tests.test_request_utils.CustomError1'),
call('error_ignored_message', 'Test failure'),
],
any_order=True
)
@override_settings(EXPECTED_ERRORS=[
@override_settings(IGNORED_ERRORS=[
{
'MODULE_AND_CLASS': 'openedx.core.lib.tests.test_request_utils.CustomError1',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
},
{
'MODULE_AND_CLASS': 'openedx.core.lib.tests.test_request_utils.CustomError2',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}
])
@patch('openedx.core.lib.request_utils.set_custom_attribute')
@@ -312,18 +310,17 @@ class TestExpectedErrorMiddleware(unittest.TestCase):
mock_first_exception = self.mock_exception
mock_second_exception = mock_first_exception if use_same_exception else CustomError2("Oops")
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, mock_first_exception)
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, mock_second_exception)
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, mock_first_exception)
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, mock_second_exception)
expected_calls = [
call('checked_error_expected_from', 'middleware'),
call('error_expected', True),
call('checked_error_ignored_from', 'middleware'),
call('error_ignored_class', 'openedx.core.lib.tests.test_request_utils.CustomError1'),
call('error_ignored_message', 'Test failure'),
call('checked_error_expected_from', 'middleware'),
call('checked_error_ignored_from', 'middleware'),
]
if use_same_exception:
expected_calls += [call('checked_error_expected_from', 'multiple')]
expected_calls += [call('checked_error_ignored_from', 'multiple')]
else:
expected_calls += [
call('unexpected_multiple_exceptions', 'openedx.core.lib.tests.test_request_utils.CustomError1'),
@@ -331,17 +328,16 @@ class TestExpectedErrorMiddleware(unittest.TestCase):
mock_set_custom_attribute.assert_has_calls(expected_calls)
assert mock_set_custom_attribute.call_count == len(expected_calls)
@override_settings(EXPECTED_ERRORS=[{
@override_settings(IGNORED_ERRORS=[{
'MODULE_AND_CLASS': 'Exception',
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}])
@patch('openedx.core.lib.request_utils.set_custom_attribute')
def test_process_exception_with_plain_exception(self, mock_set_custom_attribute):
mock_exception = Exception("Oops")
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, mock_exception)
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, mock_exception)
mock_set_custom_attribute.assert_has_calls([
call('error_expected', True),
call('error_ignored_class', 'Exception'),
call('error_ignored_message', 'Oops'),
])
@@ -354,60 +350,58 @@ class TestExpectedErrorMiddleware(unittest.TestCase):
(True, True), # log with stacktrace
)
@ddt.unpack
def test_process_exception_expected_error_with_overrides(
def test_process_exception_ignored_error_with_overrides(
self, log_error, log_stack_trace, mock_logger, mock_set_custom_attribute,
):
expected_class = 'openedx.core.lib.tests.test_request_utils.CustomError1'
expected_message = 'Test failure'
with override_settings(EXPECTED_ERRORS=[{
with override_settings(IGNORED_ERRORS=[{
'MODULE_AND_CLASS': expected_class,
'IS_IGNORED': False,
'LOG_ERROR': log_error,
'LOG_STACK_TRACE': log_stack_trace,
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}]):
ExpectedErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
IgnoredErrorMiddleware('mock-response').process_exception(self.mock_request, self.mock_exception)
if log_error:
exc_info = self.mock_exception if log_stack_trace else None
mock_logger.info.assert_called_once_with(
'Expected error %s: %s: seen for path %s', expected_class, expected_message, '/test', exc_info=exc_info
'Ignored error %s: %s: seen for path %s', expected_class, expected_message, '/test', exc_info=exc_info
)
else:
mock_logger.info.assert_not_called()
mock_set_custom_attribute.assert_has_calls(
[
call('checked_error_expected_from', 'middleware'),
call('error_expected', True),
call('checked_error_ignored_from', 'middleware'),
],
any_order=True
)
@ddt.ddt
class TestExpectedErrorExceptionHandler(unittest.TestCase):
class TestIgnoredErrorExceptionHandler(unittest.TestCase):
"""
Tests for expected_error_exception_handler.
Tests for ignored_error_exception_handler.
Note: Only smoke tests the handler to not duplicate all testing in TestExpectedErrorMiddleware.
Note: Only smoke tests the handler to not duplicate all testing in TestIgnoredErrorMiddleware.
"""
def setUp(self):
super().setUp()
RequestCache.clear_all_namespaces()
clear_cached_expected_error_settings()
clear_cached_ignored_error_settings()
self.mock_request = RequestFactory().get('/test')
self.mock_exception = CustomError1('Test failure')
@override_settings(EXPECTED_ERRORS=[{
@override_settings(IGNORED_ERRORS=[{
'MODULE_AND_CLASS': 'openedx.core.lib.tests.test_request_utils.CustomError1',
'LOG_ERROR': True,
'REASON_EXPECTED': 'Because',
'REASON_IGNORED': 'Because',
}])
@patch('openedx.core.lib.request_utils.set_custom_attribute')
@patch('openedx.core.lib.request_utils.log')
@ddt.data(True, False)
def test_handler_with_expected_error(
def test_handler_with_ignored_error(
self, use_valid_context, mock_logger, mock_set_custom_attribute
):
if use_valid_context:
@@ -416,12 +410,12 @@ class TestExpectedErrorExceptionHandler(unittest.TestCase):
else:
mock_context = None
expected_request_path = 'request-path-unknown'
expected_error_exception_handler(self.mock_exception, mock_context)
ignored_error_exception_handler(self.mock_exception, mock_context)
expected_class = 'openedx.core.lib.tests.test_request_utils.CustomError1'
expected_message = 'Test failure'
mock_logger.info.assert_called_once_with(
'Expected error %s: %s: seen for path %s',
'Ignored error %s: %s: seen for path %s',
expected_class,
expected_message,
expected_request_path,
@@ -429,8 +423,7 @@ class TestExpectedErrorExceptionHandler(unittest.TestCase):
)
mock_set_custom_attribute.assert_has_calls(
[
call('checked_error_expected_from', 'drf'),
call('error_expected', True),
call('checked_error_ignored_from', 'drf'),
call('error_ignored_class', expected_class),
call('error_ignored_message', expected_message),
],