Distributed error reporting - take 2#14870
Draft
rtibbles wants to merge 10 commits into
Draft
Conversation
Adds AdditionalSQLiteDatabaseHook, which allows a plugin to declare models that should be stored in an additional SQLite database, named after the plugin's module path. A single generic database router routes the declared models, so DATABASE_ROUTERS stays static. Hook-provided databases are resolved by get_sqlite_database_path, and included in startup foreign key checks and preseeded database copying, alongside the statically defined additional databases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
DEV: backend
Contributor
npm Package VersionsWarning The following packages have changed files but no version bump:
If these changes affect published code, consider bumping the version. |
Contributor
Build Artifacts
Smoke test screenshot |
Adds PingbackHook, dispatched from ping_once with the server and the pingback id returned by the server, so that plugins can piggyback on a successful pingback without analytics depending on them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a self-contained plugin with an ErrorReport model that stores deduplicated error reports - repeat occurrences of the same error increment an event counter rather than creating new rows. Report context is validated against per-category JSON schemas. Reports are stored in a plugin-provided SQLite database, registered via AdditionalSQLiteDatabaseHook. Reports are captured for eventual re-reporting into Sentry by the telemetry server, so each report records the data that could not be reconstructed later: - exception_type and kolibri_version, included in the report identity so that the same error before and after an upgrade is recorded separately. The version is captured at occurrence time, as reports may be submitted after an upgrade. - ServerRun records a run counter and monotonic clock reading at each server start, and reports anchor their first and last occurrences to the run as (run counter, uptime) pairs, so that occurrence times and ordering can be reconstructed independently of the device wall clock, which is unreliable on offline devices. Co-authored-by: Richard Tibbles <richard@learningequality.org> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Connects to the got_request_exception signal in the plugin's AppConfig.ready, recording each unhandled exception with its traceback and request context. The request_started signal is used to record the time from request start to error. Request information is extracted and scrubbed of sensitive data (credentials, cookies, PII headers) by utilities vendored from sentry-sdk, which safely handle request bodies that have already been consumed by Django REST Framework. Co-authored-by: Richard Tibbles <richard@learningequality.org> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Registers a PingbackHook that enqueues a task after each successful pingback, posting any unreported errors to the telemetry server with the pingback id, and marking them as reported on success. The submission payload is a versioned envelope, so that payload generations from slow-upgrading devices stay distinguishable, and includes a snapshot of the device clocks at submission time, allowing the telemetry server to calibrate the run/uptime anchors on the error reports against the time it received the submission. Co-authored-by: Richard Tibbles <richard@learningequality.org> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rtibbles
force-pushed
the
distributed-error-reporting
branch
from
June 16, 2026 22:29
ea7f86c to
39cc197
Compare
Registers a task JobHook that records an error report whenever a job transitions to the failed state, including job and worker context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a report endpoint to the error_reports plugin, accepting error reports from the frontend and storing them with the same deduplication as backend errors. Invalid reports and storage failures return a 400 rather than surfacing as server errors. Co-authored-by: Samson Akol <akolson2010@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rtibbles
force-pushed
the
distributed-error-reporting
branch
from
June 17, 2026 02:41
39cc197 to
e0ef771
Compare
Adds a frontend bundle, loaded on all pages, that captures unhandled errors and promise rejections and submits them to the plugin's report endpoint. Errors are queued with retry and deduplicated client-side, and reports include breadcrumbs of recent user interactions alongside browser, OS and device context, using screen breakpoints from KDS. Co-authored-by: Richard Tibbles <richard@learningequality.org> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The error_reports plugin is the first default plugin with migrations, so plugin updates during initialization now run migrations against every configured database - update test_first_run to allow this. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exception messages can contain sensitive data, and text interpolated into a log string cannot be filtered or redacted by logging handlers. Record the rule in the agent conventions and the code quality docs: log a static message and pass the exception via exc_info instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rtibbles
force-pushed
the
distributed-error-reporting
branch
from
June 17, 2026 03:02
e0ef771 to
b9e3b89
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
References
Closes #12214
Fixes #13578
Supersedes #12489
Companion telemetry PR: https://github.com/learningequality/nutritionfacts/pull/84
Integration test reports:
Reviewer guidance
Broadly - is the push to do Sentry pass through the right direction?
I think this strikes a good balance between giving fine grained error reports while making best efforts to remove PII - is there anything critical we might be missing?
By just doing pass through on telemetry, we are also not retaining any data on our servers, and relying on Sentry's post-report sanitization to do any further cleanup
I am happy to consider task dependencies as a better way of doing the hook handling - but it felt like a scope creep for here, so if we do want to do it, I'd rather have the analytics hook as a temporary measure, and do that as follow up.
I want to do some extra checks on the test suites to make sure they are precise and focused enough - so opening as a draft for now.
AI usage
I used Claude extensively for this in multiple separate streams of work: