fix(reminders): כשל משאיר עקבה — לוג לפני 500, "לא ידוע" בכרטיס הדשבורד, וסירוב לדחייה שנראה

Merged
AM
25d ago
+552-25

✨ תיאור קצר

שלב 4 בתוכנית התיקונים שאחרי הסקירה של #3430 (code-review-3430-sticky-reminders.md ב-Ck): WARN-009, WARN-008, WARN-005. שלושתם כשלים שקטים: 500 במסלולי התזכורות בלי שום לוג בשרת, כרטיס דשבורד שמציג 0 כשהספירה נכשלה (ומאז #3430 אפס הוא גם הערך הבריא), ודחייה מהתראה שהשרת סירב לה ונראתה למשתמש כאילו התקבלה. שני קומיטים על ענף שאותחל מ-main אחרי מיזוג #3444 (שלב 3): ed6440a השלב עצמו, ו-690f957 סבב הריוויו (סעיף משלו למטה).

📦 שינויים עיקריים

  • קוד (Backend) — webapp/sticky_notes_api.py (עוזר _failed בשבעת מסלולי התזכורות), webapp/app.py (_build_push_card)
  • קוד (Frontend) — webapp/static/sw.js (מסלול הדחייה ב-notificationclick), webapp/templates/dashboard.html (אריח "תזכורות בהמתנה"), בלי CSS
  • בוט טלגרם
  • מסד נתונים/מיגרציות
  • תיעוד (docs/)
  • טסטים (שני קבצים חדשים, ושלושה טסטים בקובץ קיים; עוד שבעה בסבב הריוויו)
  • DevOps/CI/CD

פירוט:

  • WARN-009 — לוג לפני 500. @traced רושם רק חריגה שיוצאת מהפונקציה, וה-except הגורף במסלולי התזכורות תפס אותה קודם — ולכן נתיב שנפל לא השאיר שום סימן בשרת, בזמן שהלקוח הופך את ה-500 ל-backoff שקט. הסקירה הצביעה על reminders_summary; לפי הכלל משלב 1 (כל המופעים של הדפוס באותו קובץ, לא רק המדווח) נמצאו שבעה מסלולים עם אותו except חשוף. עוזר אחד, _failed(route), רושם logger.error עם exc_info=True ומחזיר את אותו גוף Failed — החוזה ללקוח לא השתנה. מה זורם ללוג (K13): הודעה קבועה, שם המסלול, והחריגה עצמה; ה-traceback עובר את SensitiveDataFilter שמותקן על כל ה-handlers ב-webapp/app.py (install_sensitive_filter) ומנקה גם את טקסט החריגה, כמו כל exc_info אחר במודול הזה. בלי מזהה משתמש ובלי גוף בקשה. טסט מקור מבטיח שלא נשאר return חשוף במודול, כך שגם מסלול שייכתב מחר ייתפס.
  • WARN-008 — "לא ידוע" ולא 0. _build_push_card החזיר pending_count = 0 על חריגה, ומאז שהמונה נכון (#3430) זה נראה בדיוק כמו "הכול נקי". עכשיו הבונה מחזיר None ורושם אזהרה עם traceback, והתבנית מציגה "לא ידוע" עם הסבר ב-title, באותן מחלקות קיימות. שלוש הבליעות האחרות באותה פונקציה (מנויים ← 0, שליחה אחרונה ← ריק, התזכורת הבאה ← ריק) לא נגעתי בהן — הן מוצגות כ"אין מה להציג" ולא כמספר בריא, אבל "מנויים 0" על כשל נקרא כ"עוד לא הופעל", וזה מועמד לשלב 5.
  • WARN-005 — סירוב לדחייה נראה. ב-sw.js הדחייה שלחה POST ובלעה את התשובה: 404 על תזכורת שכבר אושרה (ממכשיר אחר, או מהחלונית בזמן שההתראה עוד ישבה במגש) חזר כתשובה רגילה ש-.catch אינו רואה, ו-shouldOpen פתח את הפתק כאילו הדחייה נקבעה. עכשיו response.ok נקרא; כל תוצאה מדווחת דרך reportToServer — אירוע snooze עם success/refused/error, קוד התשובה ב-http_status (לא status, שהוא שדה הדיווח עצמו) והסיבה ב-error — כדי שקצב הסירובים יהיה מדיד, וכשהדחייה לא נקבעה מוצגת התראה "הדחייה לא התקבלה" עם הפתק בנתונים, כך שלחיצה עליה פותחת אותו. הפולבק שפותח את הפתק על snooze_10 לא השתנה. בשרת אין שינוי: sw_report מקבל כל שם אירוע, שומר ומפיץ sw_push_report, שהקטלוג כבר מתאר כ"שלב בטיפול בהתראה". SW_VERSION לא הועלה, כמו ב-#3430 שגם הוא שינה את הקובץ; הדפדפן מעדכן SW לפי הבדל בבייטים.

מקורות חיצוניים שקבעו את המימוש

החלטהמקור
exc_info=True חייב להיקרא מתוך ה-exceptlogging של הספרייה הסטנדרטית: exc_info=True קורא את sys.exc_info() הפעיל — אומת בהרצה: הרשומה נושאת את RuntimeError('mongo is down')
השחרת ה-tracebackutils.SensitiveDataFilter._redact_exception (קיים), מותקן ב-webapp/app.py דרך install_sensitive_filter() — נבדק שהוא מותקן, לא נמדד כאן על הודעות pymongo
assertLogs ו-cm.recordsunittest הסטנדרטי; הרשומה נבחרת לפי שם המסלול, כי חימום האינדקסים רושם שגיאה משלו לפניה
מאפיינים על אירוע Event סינתטי (action, notification, waitUntil)אומת בהרצה ב-Chromium 141: ה-handler האמיתי קרא אותם ורץ עד הסוף (הדיווחים נלכדו)
is none בתבניתJinja2 המותקן, מבחן מובנה; אומת ברינדור העמוד האמיתי בטסט

🧪 בדיקות

  • Unit
  • Integration
  • Manual

כל טסט חדש הורץ על הקוד של היום ונפל עם הערך הישן; כל תיקון עבר בדיקת מוטציה ב-worktree בסקרצ'פאד או על עותק בזיכרון; ה-Service Worker נמדד ב-Chromium 141 דרך Playwright 1.49.0. ל-CI אין Chromium (docs/testing.rst), ולכן הראיה לטסטי הדפדפן היא הפלט המקומי שכאן.

טסטעל הקוד של היוםעל התיקון
summary נופל ← לוג שרתno logs of level ERRORרשומה sticky notes reminders_summary failed עם ה-traceback
list נופל ← לוג שרתה-ERROR היחיד היה של חימום האינדקסיםרשומה ל-reminders_list עם ה-traceback
טסט מקור: return חשוף במודול71, בתוך _failed
_build_push_card כשהספירה זורקת0None + אזהרה עם exc_info
העמוד /dashboard האמיתי עם מסד שנופל<strong>0</strong>"לא ידוע" באריח
SW, 404 לדחייה (snooze_60)אין דיווח snooze, אין התראהrefused עם http_status 404, התראה "הדחייה לא התקבלה" עם note_id, ובלי פתיחת חלון
SW, 200אין דיווחsuccess, בלי התראה
SW, שגיאת רשתאין דיווחerror עם "Failed to fetch", והתראה

מוטציות: הסרת שורת logger.error מ-_failed ← שני טסטי הלוג נופלים; pending_count = 0 בחזרה ← שני טסטי הדשבורד נופלים; ב-SW, outcome = 'success' בלי לקרוא response.ok ← בקרה קבועה בקובץ (404 מדווח כהצלחה ואין התראה). ה-SW נבדק דרך הממשק האמיתי: sw.js נטען כמו שהוא לעמוד עם דמה של registration, clients ו-fetch, ואירוע notificationclick סינתטי מפעיל את ה-handler עצמו.

  • קבצים ישירים: קובץ התזכורות 33 עברו; הדשבורד 3; ה-SW 4 יחד עם test_note_permalink (שקורא את sw.js) 17.
  • רגרסיה: 60 הקבצים שנוגעים ב-base.html, בסטיקי, בדשבורד, ב-sw.js או ב-push, יחד עם test_browser_suite_is_gated_once ושני הקבצים החדשים — 1651 עברו, 53 דולגו, אפס נפלו.
  • flake8 בבוררי ה-CI החוסמים — נקי; ב-120 הקבצים החדשים 0, sticky_notes_api.py ירד מ-15 ל-14 ממצאים ישנים, app.py ללא שינוי. ל-JS אין לינטר ב-CI. לא הורץ Prettier על התבנית.
  • מה שלא אימתתי, ומראש: רישום Service Worker אמיתי — showNotification ו-notificationclick רצו מול דמה בעמוד, לא מול רישום SW חי, כי Playwright אינו מפעיל notificationclick אמיתי; הדשבורד מול כשל מסד אמיתי — מסד מדומה; כיסוי מסנן ההשחרה על הודעות pymongo — נבדק שהוא מותקן ושהוא מנקה traceback, לא נמדד על חריגת חיבור אמיתית; בניית RTD — פרוזה בעמודים קיימים, ה-check על ה-PR בונה.

🔁 סבב ריוויו 1 (קומיט 690f957)

שלוש הערות של CodeRabbit: שתיים תוקנו מהשורש, אחת נדחתה.

  • minutes בדחייה (תוקן): int(payload.get('minutes') or 60) זרק על מחרוזת או רשימה, וה-except הגורף הפך טעות של הלקוח ל-500 — ומאז שלב 4 גם לשורת ERROR עם traceback. ההצעה הייתה לעטוף ב-except ValueError, TypeError; זה היה משאיר את int() ממיר בשקט True ל-1 ו-3.9 ל-3. במקום זה בדיקת טיפוס לפני ההמרה, לפי U3 ב-amir-bug-patterns: מספר שלם בלבד (bool מוחרג במפורש), 1 עד 1440, ואחרת 400 Invalid minutes בלי לוג. חסר או null נשאר שעה, כמתועד. שינוי חוזה קטן ומכוון: "10" כמחרוזת, 0 ו-"" (שהיו הופכים בשקט לשעה) הם עכשיו 400. הצרכן היחיד בריפו הוא ה-Service Worker, ששולח מספר שלם מהכפתור; כפתור "דחה לאחר"כ" בחלונית אינו קורא למסלול בכלל.
  • גוף שאינו אובייקט (תוקן, אותו שורש): request.get_json(silent=True) or {} מעביר רשימה (היא truthy) ואז .get זורק — 500 עם traceback במקום 400. reminders_ack כבר בדק את זה inline משלב 2; עכשיו עוזר אחד, _json_object, בשלושת מסלולי התזכורות שקוראים גוף: אין גוף ← מילון ריק (ברירות המחדל נשמרות), גוף שאינו אובייקט ← 400 invalid_payload. ב-ack הודעת ה-400 על רשימה השתנתה מ-note_id required ל-invalid_payload; הטסט הקיים בדק רק את הסטטוס. שבעה מסלולי פתקים אחרים באותו מודול משתמשים באותו or {} — לא נגעתי, מחוץ לנושא ה-PR.
  • update_one בקביעת תזכורת בלי לוג (תוקן): ה-except הפנימי החזיר Failed to save בלי עקבה — אותו כשל שקט ששלב 4 תיקן, בטקסט אחר, ולכן הספירה המילולית לא ראתה אותו. עכשיו _failed('set_note_reminder.save', error='Failed to save'): אותו חוזה ללקוח, traceback בלוג. סריקת AST חדשה בטסטים תופסת כל except במסלול תזכורות שמחזיר 500 בלי _failed או logger.error, לפי הנתיב בדקורטור ולא לפי שם הפונקציה. הסריקה מוגבלת למסלולי התזכורות בכוונה: הרצתי אותה על כל המודול, ו-13 מסלולי פתקים פולטים אירוע anomaly עם str(e) בלי traceback, ואחד (list_orphan_repos, ה-except הפנימי) שותק לגמרי — מועמדים לשלב 5, לא ל-PR הזה.
  • monkeypatch.setitem על TESTING/SECRET_KEY בפיקסטורת הדשבורד (נדחה): הפיקסטורה מציבה ישירות, כמו 29 קבצי טסט אחרים בריפו (קובץ אחד משתמש ב-setitem). הערכים זהים בכל הפיקסטורות, ולכן שחזור אחרי הטסט לא משנה שום דבר נצפה. מוסכמה בריפו, לא באג.

אימות הסבב: שבעת הטסטים החדשים הורצו על הקוד הקודם: שישה נפלו עם הערך הישן (500 ו-ERROR על "abc" ועל גוף-רשימה, note_id required ב-ack, אין רשומת לוג בכשל שמירה, set_note_reminder בסריקת ה-AST), ואחד — ברירת המחדל של שעה — עובר בשני הצדדים בכוונה, כשומר על החוזה. מוטציות ב-worktree בסקרצ'פאד: הסרת בדיקת הטיפוס ← טסט הקלט הפסול נופל; רשימה שהופכת למילון ריק ← שלושת טסטי גוף-הרשימה נופלים; החזרת ה-500 החשוף בשמירה ← טסט השמירה וסריקת ה-AST נופלים; הסרת ה-logger.error מ-_failed ← שלושת טסטי העקבה נופלים. קובץ התזכורות: 40 עברו. רגרסיה על כל קבצי הטסט שנוגעים בסטיקי, ב-api-reference או ב-USER_GUIDE, עם שני טסטי התיעוד: 996 עברו, 33 דולגו, אפס נפלו. flake8 בבוררי ה-CI נקי; ב-120 המודול נשאר על 14 ממצאים ישנים. לא אימתתי: לחיצת דחייה מ-Service Worker חי מול השרת החדש (הטסט של שלב 4 מדמה את ה-fetch), ובניית RTD — תא בטבלת list-table ומשפט בפסקה קיימת.

🧪 בדיקות נדרשות ב‑PR

  • 🔍 Code Quality & Security
  • Unit Tests (3.11)
  • Unit Tests (3.12)

📝 סוג שינוי

  • fix: תיקון באג
  • test: הוספת/עדכון בדיקות

✅ צ'קליסט

  • בדיקות רצות ועוברות
  • תיעוד עודכן
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות
  • הודעת הקומיט תואמת Conventional Commits
  • עיינתי במסמכי אתר התיעוד — נתיב: docs/observability/events_catalog.rst | המשפט: "שמרו על שמות אירועים ב-snake_case; הימנעו מ-PII" — אירוע ה-snooze של ה-SW נכנס תחת sw_push_report הקיים, בלי שם חדש בקטלוג ובלי מזהי משתמש. וגם: AI-MAP.md בתחילת העבודה, docs/dev/sticky_notes_extending.rst (עודכן), docs/user/sticky_notes.rst (עודכן), docs/testing.rst; בסבב הריוויו גם docs/doc-authoring.rst ו-docs/versioning-stable-anchors.rst לפני העריכה ב-docs/webapp/api-reference.rst. docs/webapp/theming_and_css.rst — לא נקרא, במכוון: אין CSS, האריח משתמש באותן מחלקות.

תיעוד שעודכן: docs/dev/sticky_notes_extending.rst — משפט בפסקת הדגימה על העקבה בשרת, על "לא ידוע" בכרטיס, ועל 400 בלי לוג לקלט פסול; docs/user/sticky_notes.rst — משפט בסעיף "לחצתי דחה 10 דק׳" על ההתראה כשהדחייה לא התקבלה; docs/webapp/api-reference.rst ו-webapp/USER_GUIDE.md — החוזה של minutes בדחייה.

🧩 השפעות/סיכונים

שינויי התנהגות מכוונים: כל 500 במסלולי התזכורות רושם שורת ERROR עם traceback — הנפח שווה לקצב הכשלים, וה-backoff המסלים בלקוח מקטין אותו בתקלה מתמשכת; הכרטיס מציג "לא ידוע" במקום 0 על כשל; ה-SW שולח דיווח אחד נוסף לכל לחיצת דחייה (גם על הצלחה, כדי שהקצב יימדד) ומציג התראה נוספת על סירוב או שגיאת רשת. בדחייה, minutes שאינו מספר שלם, וגוף שאינו אובייקט בשלושת מסלולי התזכורות, הם 400 ולא 500. אין שינוי בחוזה ה-API מעבר לזה.

סטיות מהתוכנית: אין. שלב 4 לא היה במצב תכנון לפי הכללים שלך (רק 2–3), ולכן ההחלטות מדווחות כאן: הרחבת הלוג לשבעת המסלולים ולא רק ל-summary; דיווח גם על הצלחת דחייה; והתראה ולא רק דיווח על סירוב. כל אחת ניתנת לשינוי במילה.

מה שנשאר פתוח: שלב 5 לפי התוכנית, ואליו מצטרפים "מנויים 0 על כשל" בכרטיס, ה-except השותק ב-list_orphan_repos ו-13 מסלולי הפתקים שפולטים אירוע בלי traceback, or {} בשבעת מסלולי הפתקים, ו-SUGG-013/SUGG-014 שמסומנים "May never fire" ודורשים בדיקת רלוונטיות מחדש.

🔗 קישורים

  • Issues קשורים: #3430 (הסקירה), #3438 (שלב 2), #3444 (שלב 3).
  • Docs Preview:
  • מסמכים/מפרטים רלוונטיים: code-review-3430-sticky-reminders.md ב-Ck — WARN-009, WARN-008, WARN-005.

🧯 סיכון / החזרה לאחור (Rollback)

Revert של שני הקומיטים מחזיר את הכל. אין מיגרציה, אין שינוי באינדקסים, אין שינוי בחוזה ה-API מעבר ל-400 על קלט פסול. ה-SW מתעדכן לפי הבדל בבייטים, גם ב-revert.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YNRfYgfyQFv3Gk9UuXDvFx

Generated by Claude Code

Summary by Sourcery

Make sticky-note reminder failures observable and distinguishable from successful, empty states across the API, dashboard, and notifications.

Bug Fixes:

  • Add server-side error logging with tracebacks for all sticky-note reminder API failures while preserving the existing 500 response contract.
  • Distinguish failed dashboard reminder counts from a healthy zero by displaying "unknown" and logging the failure.
  • Make rejected or failed notification snoozes visible to users and report snooze success, refusal, and network errors.

Documentation:

  • Update sticky-note development and user documentation to describe server traces, unknown dashboard counts, and rejected snoozes.

Tests:

  • Add coverage for reminder-route logging, dashboard unknown-count rendering, and Service Worker snooze outcomes.