From 182b247c37cc6a8982d366b5c0195247c350f99f Mon Sep 17 00:00:00 2001 From: JoaquinBN Date: Tue, 4 Aug 2026 15:57:19 +0200 Subject: [PATCH] Fix Creator and Builder role grants failing since the June onboarding rework (#968) * Surface the real reason when Creator journey completion fails Users who fail to complete the Creator journey currently hit an opaque HTML 500 that the portal can only render as a generic "try again" dead end, and the reports keep coming with nothing actionable in them. The community completion endpoint now catches unhandled errors the same way the builder endpoint does: the role grant still rolls back, the client receives the actual failure reason as JSON (which the portal already displays verbatim), and the full traceback is written to the logs. Builder journey completion logging now keeps the traceback as well instead of only the exception message. ## Claude Implementation Notes - backend/users/views.py: Wrapped complete_community_journey's transaction and success response in try/except returning a JSON 500 with the real error, logged via logger.exception (catching suppresses django.request's own 500 traceback logging). Switched complete_builder_journey's logger.error to logger.exception for the same reason. - backend/users/tests/test_community_journey.py: New test patches update_user_leaderboard_entries to raise during the POST only (setup-step signals call the same function) and asserts JSON 500 with the reason, an ERROR log, and Creator rollback. * Assert the completion failure log carries the traceback The failure test only proved an ERROR record was emitted, so reverting logger.exception to a bare logger.error(str(e)) would still pass while losing the diagnostic traceback this change exists to keep. The test now captures the log output and requires both the traceback and the original exception text in it. ## Claude Implementation Notes - backend/users/tests/test_community_journey.py: Capture the assertLogs context in test_complete_failure_returns_json_error_and_rolls_back and assert 'Traceback' and the exception message appear in the joined output. * Fix new Creator and Builder role grants failing at the final step Since the June onboarding rework, every first-time role grant through the Creator and Builder journeys failed in production while all tests passed. The completion endpoints re-fetched the user via type(request.user), and in production request.user is Django's lazy wrapper: every attribute access proxies to the real user, but the wrapper's type has no .objects manager, so the grant crashed and rolled back on the exact line that only runs for genuinely new members. Tests authenticated with a concrete user, which is why the suite never saw it. Both endpoints now resolve the model through get_user_model(), and each journey gains a completion test that runs through the real session authentication chain so the wrapper shape is covered from now on. ## Claude Implementation Notes - backend/users/views.py: Replace type(user).objects with get_user_model().objects in complete_builder_journey and complete_community_journey; comments explain the SimpleLazyObject trap. Confirmed against the production CloudWatch traceback (AttributeError: type object 'SimpleLazyObject' has no attribute 'objects' at the fresh_user re-fetch). - backend/users/tests/test_community_journey.py: test_complete_grants_creator_through_wallet_session_auth completes the journey via login_wallet_session; reproduced the production 500 before the fix. - backend/users/tests/test_builder_journey.py: test_complete_grants_role_through_wallet_session_auth, same pattern for the builder grant. - backend/CLAUDE.md: New "request.user is a lazy wrapper" section documenting the trap and the wallet-session test guards. * Update changelog * Keep exception internals out of journey completion error responses A failed role grant returned str(exception) to the client, and the portal renders those fields verbatim. Database error text can carry table and constraint names or even the conflicting row values, so the client now gets a stable completion_failed code with a friendly message while the full traceback stays in the server logs, which today's diagnosis proved is the channel that matters. ## Claude Implementation Notes - backend/users/views.py: Both journey completion except blocks return {'error': 'completion_failed', 'message': ...} instead of interpolating str(e); logger.exception unchanged. The frontend completionErrorMessage helpers pick up data.message, so no frontend change is needed. - backend/users/tests/test_community_journey.py: The failure test now asserts the stable error code and that the exception text does NOT appear anywhere in the response, while still requiring it in the logged traceback. * Show tracebacks in the dev console log format logger.exception call sites are the only durable traceback source once a view catches an error, and the production JSON formatter already renders them. The human-readable console formatter used in development dropped exc_info entirely, leaving a bare one-line ERROR. It now appends the formatted traceback, and both formatters have a direct test pinning the exception rendering. ## Claude Implementation Notes - backend/tally/middleware/logging_utils.py: LayeredFormatter.format appends self.formatException(record.exc_info) when present, matching LayeredJSONFormatter's behavior. - backend/tally/tests/test_logging_formatters.py: New direct tests asserting both formatters include the traceback and exception text. --- CHANGELOG.md | 2 + backend/CLAUDE.md | 12 ++++ backend/tally/middleware/logging_utils.py | 7 +- .../tally/tests/test_logging_formatters.py | 35 ++++++++++ backend/users/tests/test_builder_journey.py | 16 +++++ backend/users/tests/test_community_journey.py | 43 ++++++++++++ backend/users/views.py | 67 +++++++++++++------ 7 files changed, 161 insertions(+), 21 deletions(-) create mode 100644 backend/tally/tests/test_logging_formatters.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1652aad9..77164813 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable user-facing changes to this project will be documented in this file. ## Unreleased +- Finishing the Creator or Builder journey now actually grants the role: since late June the final "Become a Creator" / "Claim Builder Role" step failed for every new member with a generic error, and completion errors now show their real reason instead of a dead-end "try again" (9d546e70) + - Validators can link Telegram support groups to their validator: generate a one-time code on the new Telegram Support page, paste it in a Telegram group with the Deckard support bot, and the group is bound to the validator (multiple groups supported, codes expire in 48 hours and can be revoked) (0cd7e5f) - Marketing can create campaign links like portal.genlayer.foundation/join/builders/ethcc from the admin panel without a deployment; each link tracks visits, signups, and role activations per campaign, campaign traffic reaches Google Analytics with clean final URLs, and new accounts are attributed to the campaign that brought them (810bdc32) diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index 70f04f3f..bc5b2baa 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -682,6 +682,18 @@ The project uses **context-aware serialization** to optimize API performance: - `LightEvidenceURLTypeSerializer` - Minimal (id, name, slug, is_generic) for nested use in Evidence responses - `EvidenceURLTypeSerializer` - Full serializer with url_patterns for client-side detection, used in ContributionType responses +### request.user is a lazy wrapper (never `type(request.user)`) + +`EthereumAuthentication` returns Django's `request._request.user`, which is a +`SimpleLazyObject`. Attribute access proxies to the real User (`.pk`, `hasattr`, +serialization all work, and tests using `force_authenticate` pass because they +inject a concrete User), but `type(request.user)` returns the wrapper class, +which has no `.objects` manager. This broke every new Creator/Builder role grant +in production for five weeks while all tests stayed green. Use +`get_user_model()` for manager access, and cover session-authenticated view +paths with `ethereum_auth.testing.login_wallet_session` (guards: +`test_*_through_wallet_session_auth` in `users/tests/test_*_journey.py`). + ### Per-request user lookup (do not reintroduce the address scan) `EthereumAuthentication` (`ethereum_auth/authentication.py`) runs on EVERY DRF request: diff --git a/backend/tally/middleware/logging_utils.py b/backend/tally/middleware/logging_utils.py index 5e0b5959..1a0ab489 100644 --- a/backend/tally/middleware/logging_utils.py +++ b/backend/tally/middleware/logging_utils.py @@ -75,7 +75,12 @@ def format(self, record: logging.LogRecord) -> str: message = record.getMessage() level = record.levelname - return f"[{layer}] {level}: {message}{cid_suffix}" + formatted = f"[{layer}] {level}: {message}{cid_suffix}" + # logger.exception callers rely on the traceback reaching the output; + # the JSON formatter renders it, so the dev console must too. + if record.exc_info: + formatted = f"{formatted}\n{self.formatException(record.exc_info)}" + return formatted class LayeredJSONFormatter(logging.Formatter): diff --git a/backend/tally/tests/test_logging_formatters.py b/backend/tally/tests/test_logging_formatters.py new file mode 100644 index 00000000..01053e30 --- /dev/null +++ b/backend/tally/tests/test_logging_formatters.py @@ -0,0 +1,35 @@ +import logging +import sys + +from django.test import SimpleTestCase + +from tally.middleware.logging_utils import LayeredFormatter, LayeredJSONFormatter + + +def _record_with_exception(): + try: + raise RuntimeError('formatter boom') + except RuntimeError: + exc_info = sys.exc_info() + return logging.LogRecord( + name='tally.app.users', level=logging.ERROR, pathname=__file__, + lineno=1, msg='Failed to complete community journey', args=(), + exc_info=exc_info, + ) + + +class LayeredFormatterExceptionTest(SimpleTestCase): + """Both formatters must render exc_info: logger.exception call sites are + the only durable traceback source once a view catches the error (catching + suppresses django.request's own 500 logging).""" + + def test_console_formatter_appends_traceback(self): + output = LayeredFormatter().format(_record_with_exception()) + self.assertIn('[APP] ERROR: Failed to complete community journey', output) + self.assertIn('Traceback', output) + self.assertIn('formatter boom', output) + + def test_json_formatter_includes_traceback(self): + output = LayeredJSONFormatter().format(_record_with_exception()) + self.assertIn('Traceback', output) + self.assertIn('formatter boom', output) diff --git a/backend/users/tests/test_builder_journey.py b/backend/users/tests/test_builder_journey.py index 518990bc..76999de6 100644 --- a/backend/users/tests/test_builder_journey.py +++ b/backend/users/tests/test_builder_journey.py @@ -178,6 +178,22 @@ def test_complete_grants_role_point_free(self): LeaderboardEntry.objects.filter(user=self.user, type='builder').exists() ) + def test_complete_grants_role_through_wallet_session_auth(self): + # Same grant as test_complete_grants_role_point_free, but through the + # real session authentication chain: EthereumAuthentication returns + # Django's SimpleLazyObject wrapper, which force_authenticate cannot + # model (type(request.user) on the wrapper has no .objects manager). + from ethereum_auth.testing import login_wallet_session + self.client.force_authenticate(user=None) + login_wallet_session(self.client, self.user) + self.complete_star_task() + + response = self.client.post('/api/v1/users/complete_builder_journey/') + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + from builders.models import Builder + self.assertTrue(Builder.objects.filter(user=self.user).exists()) + def test_complete_is_idempotent(self): self.complete_star_task() diff --git a/backend/users/tests/test_community_journey.py b/backend/users/tests/test_community_journey.py index 26c91304..980f2241 100644 --- a/backend/users/tests/test_community_journey.py +++ b/backend/users/tests/test_community_journey.py @@ -281,6 +281,21 @@ def test_complete_grants_creator(self): self.assertEqual(res.status_code, status.HTTP_201_CREATED) self.assertTrue(Creator.objects.filter(user=self.user).exists()) + def test_complete_grants_creator_through_wallet_session_auth(self): + # Production resolves request.user through EthereumAuthentication, which + # returns Django's SimpleLazyObject wrapper. force_authenticate injects + # a concrete User, so it cannot catch wrapper-only breakage such as + # type(request.user) having no .objects manager. + from ethereum_auth.testing import login_wallet_session + self.client.force_authenticate(user=None) + login_wallet_session(self.client, self.user) + self.start_journey() + self.complete_steps_1_to_4() + CommunityPostProof.objects.create(user=self.user, post_url=POST_URL, tweet_id='1790000000000000000') + res = self.client.post('/api/v1/users/complete_community_journey/') + self.assertEqual(res.status_code, status.HTTP_201_CREATED) + self.assertTrue(Creator.objects.filter(user=self.user).exists()) + def test_complete_is_idempotent(self): self.start_journey() self.complete_steps_1_to_4() @@ -291,6 +306,34 @@ def test_complete_is_idempotent(self): self.assertEqual(second.status_code, status.HTTP_200_OK) self.assertEqual(Creator.objects.filter(user=self.user).count(), 1) + def test_complete_failure_returns_json_error_and_rolls_back(self): + # An unhandled exception used to produce an HTML 500 the portal can only + # render as a generic "try again"; it must come back as JSON with the + # real reason, be logged with a traceback, and roll back the role grant. + self.start_journey() + self.complete_steps_1_to_4() + CommunityPostProof.objects.create(user=self.user, post_url=POST_URL, tweet_id='1790000000000000000') + # Patched around the POST only: contribution signals in the setup steps + # call the same function. + with ( + patch('leaderboard.models.update_user_leaderboard_entries', + side_effect=RuntimeError('leaderboard exploded')), + self.assertLogs('tally.app.users', level='ERROR') as logs, + ): + res = self.client.post('/api/v1/users/complete_community_journey/') + # The log must carry the traceback, not just a message: a bare ERROR + # record would pass assertLogs while losing the diagnostic value. + logged = '\n'.join(logs.output) + self.assertIn('Traceback', logged) + self.assertIn('leaderboard exploded', logged) + self.assertEqual(res.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + # The real reason belongs in the logs only: exception text can carry + # table/constraint names or row values, and the portal shows these + # response fields verbatim. + self.assertEqual(res.data['error'], 'completion_failed') + self.assertNotIn('leaderboard exploded', str(res.data)) + self.assertFalse(Creator.objects.filter(user=self.user).exists()) + def test_existing_creator_is_grandfathered(self): # A pre-existing creator who never went through the # new journey is grandfathered in: treated as a complete member and diff --git a/backend/users/views.py b/backend/users/views.py index fe7a3bb3..b0b7fc27 100644 --- a/backend/users/views.py +++ b/backend/users/views.py @@ -596,8 +596,10 @@ def complete_builder_journey(self, request): # Recalculate leaderboard entries now that the Builder relation # exists. The grant itself adds no points, but builder-category # aggregation keys off the Builder profile being present. + # get_user_model(), never type(user): request.user is Django's + # SimpleLazyObject wrapper, whose type has no .objects manager. from leaderboard.models import update_user_leaderboard_entries - fresh_user = type(user).objects.get(pk=user.pk) + fresh_user = get_user_model().objects.get(pk=user.pk) update_user_leaderboard_entries(fresh_user) serializer = self.get_serializer(user) @@ -606,10 +608,17 @@ def complete_builder_journey(self, request): 'user': serializer.data }, status=status.HTTP_201_CREATED) - except Exception as e: - logger.error(f"Failed to complete builder journey: {str(e)}") + except Exception: + # logger.exception keeps the traceback: catching the error here + # suppresses django.request's own 500 logging. The client gets a + # stable message only; str(e) can leak table/constraint names and + # row values, and the portal renders these fields verbatim. + logger.exception('Failed to complete builder journey') return Response( - {'error': f'Failed to complete journey: {str(e)}'}, + { + 'error': 'completion_failed', + 'message': 'Something went wrong on our side while completing the journey. Please try again in a moment.', + }, status=status.HTTP_500_INTERNAL_SERVER_ERROR ) @@ -869,25 +878,43 @@ def complete_community_journey(self, request): from django.db import transaction from django.contrib.auth import get_user_model - with transaction.atomic(): - # Lock the user row so two concurrent requests can't both pass the - # hasattr check above and race into a OneToOne IntegrityError. - get_user_model().objects.select_for_update().get(pk=user.pk) - _, created = Creator.objects.get_or_create(user=user) + try: + with transaction.atomic(): + # Lock the user row so two concurrent requests can't both pass the + # hasattr check above and race into a OneToOne IntegrityError. + get_user_model().objects.select_for_update().get(pk=user.pk) + _, created = Creator.objects.get_or_create(user=user) - if not created: - return Response( - {'message': 'You are already a creator', 'user': self.get_serializer(user).data}, - status=status.HTTP_200_OK, - ) + if not created: + return Response( + {'message': 'You are already a creator', 'user': self.get_serializer(user).data}, + status=status.HTTP_200_OK, + ) - fresh_user = type(user).objects.get(pk=user.pk) - update_user_leaderboard_entries(fresh_user) + # get_user_model(), never type(user): request.user is Django's + # SimpleLazyObject wrapper, whose type has no .objects manager. + fresh_user = get_user_model().objects.get(pk=user.pk) + update_user_leaderboard_entries(fresh_user) - return Response( - {'message': 'Welcome to the GenLayer community!', 'user': self.get_serializer(fresh_user).data}, - status=status.HTTP_201_CREATED, - ) + return Response( + {'message': 'Welcome to the GenLayer community!', 'user': self.get_serializer(fresh_user).data}, + status=status.HTTP_201_CREATED, + ) + except Exception: + # An unhandled error here surfaces as an HTML 500 the portal can + # only render as a generic "try again" dead end. Catching it drops + # django.request's traceback logging, so log it ourselves. The + # client gets a stable message only; str(e) can leak table or + # constraint names and row values, and the portal renders these + # fields verbatim. + logger.exception('Failed to complete community journey') + return Response( + { + 'error': 'completion_failed', + 'message': 'Something went wrong on our side while completing the journey. Please try again in a moment.', + }, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) @action(detail=False, methods=['get']) def validators(self, request):