feat: persist enketo URLs in MetaData and improve error handling by FrankApiyo · Pull Request #3031 · onaio/onadata

FrankApiyo · 2026-03-19T14:58:29Z

Summary

Enhance the existing /enketo endpoint on XFormViewSet to persist enketo URLs in the MetaData table (database) and serve them on subsequent requests without hitting the Enketo API. Also improve error handling for Enketo API failures.

On first request, call the Enketo API and store results in MetaData; subsequent requests are served directly from the database
An optional cache layer (using Django's cache framework — works with any configured backend, not Redis-specific) accelerates repeated reads; the primary source of truth is always the MetaData table
Add survey_type query param: "single" for single-submit URL only, "preview" for preview URL only, omit for all URLs
Skip persistence when form-default query params are present (each combination produces unique URLs)
Default get_form_url protocol param to ENKETO_PROTOCOL setting instead of hardcoded "https"
Return HTTP 400 for all Enketo API errors; handle_enketo_error prefixes messages with "Enketo error: " and appends the Sentry event reference for traceability
For Enketo 400 responses, the upstream error message is preserved in the response
For Enketo 500/502 and other non-400 responses, a generic retry message is returned to avoid leaking upstream details
Use e.message instead of str(e) to avoid CodeQL information-exposure flag
report_exception now returns the Sentry event_id for traceability

Test plan

Tested manually on localhost
Existing enketo tests updated and passing
New test for Sentry event_id reference in 400 error responses
Verify enketo endpoint returns stored URLs on second call without hitting Enketo API
Verify form-default requests always call the Enketo API
Verify survey_type=preview returns only the preview URL
Verify survey_type=single returns only the single-submit URL

onadata/apps/api/viewsets/xform_viewset.py

ukanga

Overall, this seems to introduce a direct dependency on Redis. If someone chooses a different cache backend, like Memcached, they will still need to use the Redis server for Enketo URLs. I think we should stick to the default cache and not have Redis-specific functionality just for Enketo URLs.

If the goal is to import existing URLs, can we consider a different import approach?

onadata/apps/api/tests/viewsets/test_xform_viewset.py

onadata/apps/api/viewsets/xform_viewset.py

…RL protocol - Add enketo_redis module for caching survey URLs using same key patterns as Zebra - Default get_form_url protocol param to ENKETO_PROTOCOL setting instead of hardcoded "https" - Pass protocol and generate_consistent_urls in data_viewset and update_enketo_urls command - Add ENKETO_LINKS_REDIS_URL setting to enable/disable Redis caching - Add tests for enketo_redis caching module

…d fix pylint issues Remove manual ConnectionPool management (redis.Redis handles pooling internally) and create a fresh client per call. Narrow broad exception catches to specific Redis/OS errors and eliminate global statement.

- Fix f-string in _() translation calls so makemessages can extract strings - Store both survey URLs hash and preview string on every cache miss - Add delete_cached_urls() and call it on form deletion - Remove redundant protocol=settings.ENKETO_PROTOCOL from callers - Default VERIFY_SSL to False - Group ENKETO_LINKS_REDIS_URL with other ENKETO_* settings

Log EnketoError details server-side and return a generic message to the client, fixing the CodeQL information-exposure-through-exception finding.

- Cache the redis.Redis client at module level since it manages its own ConnectionPool internally — avoids creating a new TCP connection per call - Restore VERIFY_SSL = True (was accidentally changed to False) - Add ValueError/TypeError to except tuples so non-numeric form_pk values are caught and logged instead of raising a 500

…to_urls _fetch_enketo_urls now raises EnketoError/ParseError instead of returning a Response on failure, eliminating the isinstance() check in the caller. Also reset enketo_redis._client in test setUp/tearDown to prevent cached clients leaking across test classes.

…nketo errors get_form_url now defaults to ENKETO_PROTOCOL from settings, so explicit callers in logger/views.py and xform_serializer.py no longer need to pass it. Also change enketo_links EnketoError response from 400 to 502 since the failure is an upstream service issue, not a client error.

Tests were asserting the old verbose error messages that leaked Enketo exception details. Updated to match the new generic "Enketo error, please retry." message.

The preview_url is already stored in the enketo-survey-urls hash, so the separate enketo-preview-url string key is unnecessary duplication. Both show_preview and full-links requests now read from the single hash.

Merge Redis caching, show_preview param, single_once_url preference, and 502 error codes into the existing /enketo endpoint instead of maintaining a separate /enketo-links endpoint. Caching is skipped when form-default query params are present.

Move show_preview/survey_type response branching into a static helper to keep enketo() under pylint's max-return-statements threshold.

Use the existing MetaData table as the single source of truth for enketo URLs instead of a separate Redis hash. On the first request the Enketo API is called and results are stored in MetaData; subsequent requests read directly from the database. - Remove enketo_redis.py module and its tests - Remove ENKETO_LINKS_REDIS_URL and ENKETO_LINKS_CACHE_TTL settings - Add _read_enketo_metadata and _store_enketo_metadata helpers - Remove delete_cached_urls call from destroy

Replace show_preview=true with survey_type=preview for consistency. The survey_type param now supports three values: omitted (all URLs), "single" (single-submit only), "preview" (preview only).

Publishing a form populates MetaData with enketo URLs via the serializer. The error tests need to clear these entries so the /enketo endpoint falls through to the Enketo API where the error mocks are applied.

Enketo 400 responses now produce HTTP 500 in our API with the message prefixed by "Enketo error: ". Enketo 500/502 responses continue to return 502 with the friendly retry message. Adds status_code attribute to EnketoError so callers can use the intended HTTP status.

CodeQL flagged str(e) as potential stack trace exposure. Using the explicit e.message attribute returns the same controlled value without triggering the scanner.

report_exception now returns the Sentry event_id. When Enketo returns a 400, the error message appends a (reference: <event_id>) suffix so users and support can trace the issue in Sentry.

dict.get() eagerly evaluates its default argument. When the mock response has str content instead of bytes, response.text triggers a chardet TypeError. Use conditional expression instead so response.text is only accessed when the JSON lacks a "message" key.

The fallback path in handle_enketo_error was passing raw response.text to EnketoError, which could expose upstream server details to API consumers (CodeQL information-exposure flag). Now returns a generic message with Sentry event_id reference for all non-400 errors.

… code changes Cache all three enketo URLs (enketo_url, enketo_preview_url, enketo_single_submit_url) as a single composite dict in Redis, eliminating DB queries on cache hit and consolidating 3 separate queries into 1 on cache miss. Add signal-based cache invalidation when enketo MetaData is saved or deleted. Also reverts EnketoError from APIException back to Exception and restores HTTP 400 for all enketo error responses, keeping the improved error messages (Sentry references, no upstream detail leakage).

Consolidate duplicate cache_tools import and replace unnecessary dict comprehension with dict() call.

Ensures every error response from the /enketo endpoint starts with the 'Enketo error:' prefix. Also includes the Sentry event_id reference in all error paths where it is available, including the >= 500 JSON parse failure path.

…e, fix docs Move the 'Enketo error:' prefix from handle_enketo_error to each caller so callers that already add their own prefix (logger/views, main/views) no longer produce double-prefixed messages. Make ENKETO_URLS_CACHE_TTL overridable via Django settings instead of hardcoded 24h. Update docs to show HTTP 400 for all Enketo errors, matching the actual implementation.

…t mocks, revert CI noise - Extract ENKETO_GENERIC_ERROR constant and _enketo_error_msg helper to DRY up handle_enketo_error - Fix double space in user-facing error message - Make ENKETO_URLS_CACHE_TTL lazy via get_enketo_urls_cache_ttl() so @override_settings works in tests - Make enketo_error403_mock return dict (consistent with 500/502 mocks) - Fix test_get_form_url to pass explicit protocol for HTTPS assertions - Revert cosmetic CI YAML whitespace changes

Add _normalize_enketo_api_keys to map Enketo API response keys (offline_url, preview_url, single_url) to canonical keys (enketo_url, enketo_preview_url, single_submit_url) once, so _store_enketo_metadata and _format_enketo_response only deal with one naming convention.

The PR description states handle_enketo_error prefixes messages with "Enketo error: " but the prefix was applied in the viewset catch block. Move it into _enketo_error_msg so the boundary function owns the formatting. Also fix test_get_form_url assertions that expected https while ENKETO_PROTOCOL was overridden to http.

kelvin-muchiri · 2026-03-26T13:08:06Z

onadata/libs/utils/viewer_tools.py

+            event_id = report_exception(f"HTTP Error {response.status_code}", message)
+        raise EnketoError(_enketo_error_msg(message, event_id))
+
+    if not event_id:


Looks like we can move if not event_id block above if response.status_code == 400 and get rid of the one inside if response.status_code == 400

Hoist the report_exception call above the status_code branch since it runs unconditionally when event_id is falsy. Also fix flaky test ordering by adding .order_by("pk") to the project queryset.

Cover all branches: valid/invalid JSON, 400/5xx/other status codes, presence/absence of message key, and Enketo error prefix.

FrankApiyo force-pushed the feat/enketo-links-redis-caching branch from c96fb8b to d7f3c75 Compare March 23, 2026 06:41

github-advanced-security bot found potential problems Mar 23, 2026

View reviewed changes

onadata/apps/api/viewsets/xform_viewset.py Fixed Show fixed Hide fixed

FrankApiyo force-pushed the feat/enketo-links-redis-caching branch from 098cc20 to 82468bf Compare March 23, 2026 07:33

FrankApiyo requested review from kelvin-muchiri and ukanga March 23, 2026 08:57

FrankApiyo force-pushed the feat/enketo-links-redis-caching branch from 52a5d63 to 1f6e2ca Compare March 23, 2026 12:06

ukanga reviewed Mar 23, 2026

View reviewed changes

onadata/apps/api/tests/viewsets/test_xform_viewset.py Outdated Show resolved Hide resolved

onadata/apps/api/tests/viewsets/test_xform_viewset.py Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Mar 23, 2026

View reviewed changes

onadata/apps/api/viewsets/xform_viewset.py Fixed Show fixed Hide fixed

FrankApiyo changed the title ~~feat: add Redis caching for enketo survey links and consistent form URL protocol~~ feat: persist enketo URLs in MetaData and improve error handling Mar 23, 2026

FrankApiyo requested a review from ukanga March 23, 2026 16:32

FrankApiyo added 20 commits March 24, 2026 09:57

fix: prevent exception details leaking to API responses

dc537b2

Log EnketoError details server-side and return a generic message to the client, fixing the CodeQL information-exposure-through-exception finding.

fix: update tests to expect generic enketo error message

ee473de

Tests were asserting the old verbose error messages that leaked Enketo exception details. Updated to match the new generic "Enketo error, please retry." message.

refactor: remove redundant enketo-preview-url Redis key

7ba1880

The preview_url is already stored in the enketo-survey-urls hash, so the separate enketo-preview-url string key is unnecessary duplication. Both show_preview and full-links requests now read from the single hash.

refactor: extract _format_enketo_response to reduce return statements

8f71eca

Move show_preview/survey_type response branching into a static helper to keep enketo() under pylint's max-return-statements threshold.

Update ECR image-build.yaml file

d9e6b7d

fix: revert to using single_url instead of single_once_url

a665e00

refactor: unify survey_type param, remove show_preview

b207139

Replace show_preview=true with survey_type=preview for consistency. The survey_type param now supports three values: omitted (all URLs), "single" (single-submit only), "preview" (preview only).

fix: clear MetaData in error tests so endpoint hits Enketo API

4d239d0

Publishing a form populates MetaData with enketo URLs via the serializer. The error tests need to clear these entries so the /enketo endpoint falls through to the Enketo API where the error mocks are applied.

fix: use e.message instead of str(e) to avoid CodeQL info-exposure flag

5c6369b

CodeQL flagged str(e) as potential stack trace exposure. Using the explicit e.message attribute returns the same controlled value without triggering the scanner.

feat: include Sentry event_id in Enketo 400 error responses

77aab52

report_exception now returns the Sentry event_id. When Enketo returns a 400, the error message appends a (reference: <event_id>) suffix so users and support can trace the issue in Sentry.

Add support for the legacy 'show_preview=true' param

0590562

FrankApiyo force-pushed the feat/enketo-links-redis-caching branch from b4200b0 to 0590562 Compare March 24, 2026 06:57

FrankApiyo added 3 commits March 24, 2026 12:11

fix: resolve prospector warnings in xform_viewset

b5197a3

Consolidate duplicate cache_tools import and replace unnecessary dict comprehension with dict() call.

FrankApiyo force-pushed the feat/enketo-links-redis-caching branch from 0708d1a to b5197a3 Compare March 26, 2026 07:57

FrankApiyo added 5 commits March 26, 2026 11:00

Merge branch 'main' into feat/enketo-links-redis-caching

100c839

fix: wrap long line to satisfy E501

6ebc19d

FrankApiyo force-pushed the feat/enketo-links-redis-caching branch from e40ea54 to 310ccb4 Compare March 26, 2026 11:16

FrankApiyo added 6 commits March 26, 2026 14:17

fix: revert cosmetic CI YAML whitespace changes

4495187

fix: resolve prospector E501 and E305 warnings

ec58119

fix: add maintenance comment on known_params allowlist

1657a27

docs: use realistic enketo URL examples in API documentation

9712c66

kelvin-muchiri reviewed Mar 26, 2026

View reviewed changes

FrankApiyo added 2 commits March 26, 2026 16:52

refactor: deduplicate report_exception call in handle_enketo_error

9176dd6

Hoist the report_exception call above the status_code branch since it runs unconditionally when event_id is falsy. Also fix flaky test ordering by adding .order_by("pk") to the project queryset.

test: add branch coverage for handle_enketo_error

eaded05

Cover all branches: valid/invalid JSON, 400/5xx/other status codes, presence/absence of message key, and Enketo error prefix.

kelvin-muchiri approved these changes Mar 26, 2026

View reviewed changes

FrankApiyo merged commit 2df1fcd into main Mar 30, 2026
11 checks passed

FrankApiyo deleted the feat/enketo-links-redis-caching branch March 30, 2026 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: persist enketo URLs in MetaData and improve error handling#3031

feat: persist enketo URLs in MetaData and improve error handling#3031
FrankApiyo merged 37 commits intomainfrom
feat/enketo-links-redis-caching

FrankApiyo commented Mar 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

ukanga left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kelvin-muchiri Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

FrankApiyo commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Uh oh!

ukanga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kelvin-muchiri Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

FrankApiyo commented Mar 19, 2026 •

edited

Loading