feat: better error handling with daily articles

Author: julienc91

backend/caviardeul/management/commands/check_next_article.py

@@ -16,9 +16,6 @@ def handle(self, *args, **options):
             raise CommandError("No daily article left")
 
         try:
-            title, text = get_article_html_from_wikipedia(next_article.page_id)
+            get_article_html_from_wikipedia(next_article.page_id)
         except ArticleFetchError:
             raise CommandError("Error when retrieving daily article")
-
-        if "redirectMsg" in text:
-            raise CommandError("Next daily article has a redirect")

backend/caviardeul/serializers/error.py

@@ -0,0 +1,5 @@
+from ninja import Schema
+
+
+class ErrorSchema(Schema):
+    detail: str

backend/caviardeul/services/articles.py

@@ -53,14 +53,19 @@ def get_article_html_from_wikipedia(page_id: str) -> tuple[str, str]:
         },
     )
     if response.status_code != 200:
-        raise ArticleFetchError
+        raise ArticleFetchError(f"Unexected response from API: {response.status_code}")
 
     data = response.json()
     if "error" in data:
-        raise ArticleFetchError
+        raise ArticleFetchError("Error received in API response")
 
     data = data["parse"]
-    return data["title"], data["text"]
+    html_content = data["text"]
+
+    if 'class="redirectMsg"' in html_content:
+        raise ArticleFetchError("Redirection received in article payload")
+
+    return data["title"], html_content
 
 
 def _prepare_article_content_from_html(page_title: str, html_content: str) -> str:

backend/caviardeul/services/daily_article.py

@@ -0,0 +1,10 @@
+from django.utils import timezone
+
+from caviardeul.models import DailyArticle
+
+
+def get_current_daily_article_id() -> int | None:
+    article = (
+        DailyArticle.objects.filter(date__lt=timezone.now()).order_by("-date").first()
+    )
+    return article.id if article else None

backend/caviardeul/tests/conftest.py

@@ -41,6 +41,36 @@ def inner(
     return inner
 
 
+@pytest.fixture()
+def mock_wiki_api_redirect(httpx_mock):
+    def inner(page_id: str, title: str):
+        httpx_mock.add_response(
+            url=httpx.URL(
+                "https://fr.wikipedia.org/w/api.php",
+                params={
+                    "action": "parse",
+                    "format": "json",
+                    "prop": "text",
+                    "formatversion": 2,
+                    "origin": "*",
+                    "page": page_id,
+                },
+            ),
+            json={
+                "parse": {
+                    "title": title,
+                    "text": (
+                        '<div class="redirectMsg"><p>Rediriger vers :</p>'
+                        '<ul class="redirectText"><li><a href="/wiki/foo" title="Foo">Foo</a></li></ul>'
+                        "</div>"
+                    ),
+                }
+            },
+        )
+
+    return inner
+
+
 @pytest.fixture()
 def mock_wiki_api_error(httpx_mock):
     def inner(page_id: str):

backend/caviardeul/tests/management/commands/test_check_next_article.py

@@ -50,7 +50,7 @@ def test_check_next_article_has_redirect(mock_wiki_api):
         '<ul class="redirectText"><li><a href="/wiki/foo" title="Foo">Foo</a></li></ul></div>',
     )
 
-    with pytest.raises(CommandError, match="Next daily article has a redirect"):
+    with pytest.raises(CommandError, match="Error when retrieving daily article"):
         call_command("check_next_article")
 
 

backend/caviardeul/tests/views/test_daily_article.py

@@ -1,3 +1,4 @@
+import logging
 from collections import defaultdict
 from datetime import datetime
 from typing import Literal
@@ -110,11 +111,25 @@ def test_get_current_article(
             "nbWinners": article.nb_winners,
         }
 
+    def test_error_with_article(self, client, caplog, mock_wiki_api_redirect):
+        article = DailyArticleFactory(trait_current=True)
+        mock_wiki_api_redirect(article.page_id, article.page_name)
+
+        with caplog.at_level(logging.ERROR):
+            res = client.get("/articles/current")
+        assert "Redirection received in article payload" in caplog.text
+
+        assert res.status_code == 500, res.content
+        data = res.json()
+        assert data["detail"]
+
     def test_no_article_available(self, client):
         _ = DailyArticleFactory(trait_future=True)
 
         res = client.get("/articles/current")
         assert res.status_code == 404, res.content
+        data = res.json()
+        assert data["detail"]
 
 
 class TestGetAchivedArticle:

backend/caviardeul/views/daily_article.py

@@ -1,12 +1,13 @@
 from datetime import timedelta
 
 from django.contrib.auth.models import AnonymousUser
-from django.db.models import Avg, Count, FilteredRelation, Q
-from django.http import Http404, HttpRequest
+from django.db.models import Avg, Count, FilteredRelation, Q, QuerySet
+from django.http import HttpRequest
 from django.utils import timezone
 from ninja import Query
 from ninja.pagination import paginate
 
+from caviardeul.exceptions import ArticleFetchError
 from caviardeul.models import DailyArticle, User
 from caviardeul.serializers.daily_article import (
     DailyArticleListFilter,
@@ -15,12 +16,29 @@
     DailyArticleSchema,
     DailyArticlesStatsSchema,
 )
+from caviardeul.serializers.error import ErrorSchema
 from caviardeul.services.articles import get_article_content
 from caviardeul.services.authentication import OptionalAPIAuthentication
+from caviardeul.services.daily_article import get_current_daily_article_id
+from caviardeul.services.logging import logger
 
 from .api import api
 
 
+@api.get(
+    "/articles/stats",
+    auth=OptionalAPIAuthentication(),
+    response=DailyArticlesStatsSchema,
+)
+def get_daily_article_stats(request: HttpRequest):
+    return _get_queryset(request.auth).aggregate(
+        total=Count("id"),
+        total_finished=Count("id", filter=Q(user_score__isnull=False)),
+        average_nb_attempts=Avg("user_score__nb_attempts"),
+        average_nb_correct=Avg("user_score__nb_correct"),
+    )
+
+
 def _get_queryset(user: User | AnonymousUser):
     queryset = DailyArticle.objects.filter(date__lte=timezone.now())
     if not user.is_authenticated:
@@ -35,44 +53,41 @@ def _get_queryset(user: User | AnonymousUser):
 
 
 @api.get(
-    "/articles/current", auth=OptionalAPIAuthentication(), response=DailyArticleSchema
-)
-def get_current_article(request: HttpRequest):
-    try:
-        article = _get_queryset(request.auth).order_by("-date")[:1].get()
-    except DailyArticle.DoesNotExist:
-        raise Http404("L'article n'a pas été trouvé")
-
-    article.content = get_article_content(article)
-    return article
-
-
-@api.get(
-    "/articles/stats",
+    "/articles/current",
     auth=OptionalAPIAuthentication(),
-    response=DailyArticlesStatsSchema,
+    response={200: DailyArticleSchema, 404: ErrorSchema, 500: ErrorSchema},
 )
-def get_daily_article_stats(request: HttpRequest):
-    return _get_queryset(request.auth).aggregate(
-        total=Count("id"),
-        total_finished=Count("id", filter=Q(user_score__isnull=False)),
-        average_nb_attempts=Avg("user_score__nb_attempts"),
-        average_nb_correct=Avg("user_score__nb_correct"),
+def get_current_article(request: HttpRequest):
+    article_id = get_current_daily_article_id()
+    return _get_daily_article_response(
+        _get_queryset(request.auth).filter(id=article_id)
     )
 
 
 @api.get(
     "/articles/{article_id}",
     auth=OptionalAPIAuthentication(),
-    response=DailyArticleSchema,
+    response={200: DailyArticleSchema, 404: ErrorSchema, 500: ErrorSchema},
 )
 def get_archived_article(request: HttpRequest, article_id: int):
+    return _get_daily_article_response(
+        _get_queryset(request.auth).filter(id=article_id)
+    )
+
+
+def _get_daily_article_response(queryset: QuerySet[DailyArticle]):
     try:
-        article = _get_queryset(request.auth).get(id=article_id)
+        article = queryset.get()
     except DailyArticle.DoesNotExist:
-        raise Http404("L'article n'a pas été trouvé")
+        return 404, {"detail": "L'article n'a pas été trouvé"}
 
-    article.content = get_article_content(article)
+    try:
+        article.content = get_article_content(article)
+    except ArticleFetchError:
+        logger.exception(
+            "Error encountered with daily article", extra={"article_id": article.id}
+        )
+        return 500, {"detail": "Un problème a été rencontré avec cet article"}
     return article
 
 

frontend/components/utils/error.tsx

@@ -0,0 +1,15 @@
+import React from "react";
+import Error from "next/error";
+
+
+const CustomError: React.FC<{ statusCode?: number, text?: string }> = ({statusCode, text}) => {
+  return (
+    <main>
+      <div className="error">
+        <Error statusCode={statusCode ?? 500} title={text ?? "Une erreur est survenue"}/>
+      </div>
+    </main>
+  )
+}
+
+export default CustomError

frontend/lib/article.ts

@@ -1,5 +1,6 @@
 import { ArticleId, EncodedArticle } from "@caviardeul/types";
 import { API_URL } from "@caviardeul/utils/config";
+import {APIError} from "@caviardeul/lib/queries";
 
 export const getEncodedArticle = async (
   articleId?: ArticleId,
@@ -23,7 +24,7 @@ export const getEncodedArticle = async (
 
   const data = await res.json();
   if (!res.ok) {
-    throw data.detail;
+    throw new APIError(res.status, data.details ?? "")
   }
 
   return {