Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: catch json errors on empty responses from lrs #456

Closed
wants to merge 6 commits into from
Closed

fix: catch json errors on empty responses from lrs #456

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c650d4a
fix: catch json errors on empty responses from ralph
Ian2012 Oct 9, 2024
a805599
perf: add lru cache for users and courses
Ian2012 Oct 11, 2024
db2444e
fix: do not raise bulk event sync exception
Ian2012 Oct 11, 2024
f72ad3f
fix: do not send duplicated events in batch
Ian2012 Oct 11, 2024
d976a2a
perf: add lru to get_anonymous_user_id instead of get_user
Ian2012 Oct 11, 2024
c8e6f40
fix: send chunk_size instead of manually handling it
Ian2012 Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions event_routing_backends/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from celery.utils.log import get_task_logger
from celery_utils.persist_on_failure import LoggedPersistOnFailureTask
from django.conf import settings

from json.decoder import JSONDecodeError
from event_routing_backends.processors.transformer_utils.exceptions import EventNotDispatched
from event_routing_backends.utils.http_client import HttpClient
from event_routing_backends.utils.xapi_lrs_client import LrsClient
Expand Down Expand Up @@ -131,7 +131,7 @@ def bulk_send_events(task, events, router_type, host_config):
client_class
)
)
except EventNotDispatched as exc:
except (EventNotDispatched, JSONDecodeError) as exc:
logger.exception(
'Exception occurred while trying to bulk dispatch {} events using client: {}'.format(
len(events),
Expand Down
2 changes: 1 addition & 1 deletion event_routing_backends/utils/xapi_lrs_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def bulk_send(self, statement_data):
response = self.lrs_client.save_statements(statement_data)

if not response.success:
if response.response.code == 409:
if response.response.code == 409 or response.response.code == 204:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just do in (204, 409) here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is never reached as the json decode error is triggered first. I will remove it

logger.warning(f"Duplicate event id found in: {response.request.content}")
else:
logger.warning(f"Failed request: {response.request.content}")
Expand Down
Loading