diff --git a/docs/environment-variables.rst b/docs/environment-variables.rst index 1fa1a01f2..d39605ded 100644 --- a/docs/environment-variables.rst +++ b/docs/environment-variables.rst @@ -76,6 +76,12 @@ - ``5000`` - ``10000`` - Bot/WebApp + * - ``MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS`` + - צינון לפני ניסיון התחברות חוזר ל-MongoDB אחרי כשל (שניות); 0 מכבה + - לא + - ``30`` + - ``30`` + - Bot/WebApp/Scripts * - ``MONGODB_RETRY_WRITES`` - הפעלת Retry לכתיבות (retryWrites) - לא diff --git a/docs/testing.rst b/docs/testing.rst index e0717d5a7..008796548 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -167,8 +167,16 @@ Mocking HTTP ב‑github_menu_handler - **האם אינדקס בשימוש** — רק ``explain`` עונה. סטאב מחזיר תוצאה נכונה גם כששאילתה סורקת את כל האוסף. - **סמנטיקה של BSON** — ``datetime`` נקטם למילישניות, ובלי ``tz_aware=True`` הוא חוזר נאיבי. השוואה בין נאיבי ל-aware זורקת ``TypeError``, ואם היא עטופה ב-``except`` — הבדיקה שנשענת עליה מפסיקה לרוץ בשקט. - **צינורות aggregation** — סטאב שנכתב ביד מבין רק את הצורה שנכתבה בו, ולכן שגיאת תחביר אמיתית עוברת אצלו. +- **בייטים מול תווים** — ``$substrBytes`` ו-``$strLenBytes`` מודדים בבייטים, ``$substrCP`` ו-``$strLenCP`` בתווים, ו-``$regexFind`` מחזיר ``idx`` **בתווים**. על טקסט עברי ערבוב היחידות חותך באמצע אות ומונגו זורקת. אף סטאב בריפו אינו מדגמן את זה — ``tests/_fake_mongo.py`` אפילו אין בו ``aggregate``. -הבדיקות האלה חיות ב-``tests/test_note_boards_mongo.py``. הן **מדלגות** כשאין ``MONGODB_URL`` או כשהשרת אינו נגיש, כך שהרצה מקומית רגילה נשארת מהירה. אותו דילוג-על-שרת-לא-נגיש קיים גם בפיקסצ'ר ``wired_mongo`` שב-``tests/conftest.py``, ומשרת את הבדיקות שמריצות את הראוטים של הוובאפ מול מסד אמיתי. +הבדיקות האלה חיות בשני קבצים, וכל אחד מהם נשען על **משתנה סביבה אחר**: + +- ``tests/test_note_boards_mongo.py`` — ``MONGODB_URL``, דרך ``pytestmark`` שנבדק פעם אחת בטעינת המודול. +- ``tests/test_snippet_hebrew_offsets_mongo.py`` — ``NOTE_FONTS_TEST_MONGO_URI``, דרך הפיקסצ'ר ``wired_mongo`` שב-``tests/conftest.py``. אותו פיקסצ'ר משרת גם את שאר הבדיקות שמריצות את הראוטים של הוובאפ מול מסד אמיתי. + +**המשתנה הנפרד אינו כפילות מיותרת.** ``tests/conftest.py`` עושה ``os.environ.setdefault('MONGODB_URL', …)`` בטעינה, כלומר המשתנה הזה **תמיד** מוגדר בבדיקות — לערך דמה. פיקסצ'ר שהיה נופל אליו היה מחכה 30 שניות לכתובת שאין מאחוריה שרת, בכל בדיקה, ואז נכשל — ו-``--maxfail=1`` היה עוצר את כל החבילה. + +שניהם **מדלגים** כשהמשתנה שלהם ריק או כשהשרת אינו נגיש, כך שהרצה מקומית רגילה נשארת מהירה. .. warning:: **הן אינן רצות ב-CI כרגע.** הג'וב ``Unit Tests`` אמנם מרים ``mongo:6.0`` כשירות, אבל הוא ``runs-on: ubuntu-latest`` **בלי** ``container:``, והשירות מוגדר **בלי** ``ports:``. לפי `תיעוד GitHub Actions `_, גישה לפי שם השירות עובדת רק כשהג'וב עצמו רץ בקונטיינר; אחרת צריך למפות פורטים ולפנות ל-``127.0.0.1:``. בלי זה המארח ``mongodb`` אינו נפתר כלל (``[Errno -3] Temporary failure in name resolution``), והבדיקות מדלגות בשקט. @@ -181,8 +189,11 @@ Mocking HTTP ב‑github_menu_handler MONGODB_URL='mongodb://127.0.0.1:27017' pytest tests/test_note_boards_mongo.py -v + NOTE_FONTS_TEST_MONGO_URI='mongodb://127.0.0.1:27017' \ + pytest tests/test_snippet_hebrew_offsets_mongo.py -v + .. warning:: - כל הרצה יוצרת מסד עם שם ייחודי משלה (תחילית ``codebot_notes_it_``), וה-teardown מוודא שהשם תואם לתחילית **לפני** ``drop_database``. אל תכוונו את ``MONGODB_URL`` למסד שיש בו נתונים אמיתיים. + שני הקבצים יוצרים מסד ייעודי משלהם ואינם נוגעים במסד ברירת המחדל: ``test_note_boards_mongo.py`` מגריל שם עם התחילית ``codebot_notes_it_``, ו-``wired_mongo`` בונה ``cktest_<שם קובץ הבדיקה>``. ה-teardown של הראשון מוודא שהשם תואם לתחילית **לפני** ``drop_database``. עם זאת — אל תכוונו את אף אחד משני המשתנים למסד שיש בו נתונים אמיתיים. כיסוי בדיקות (pytest-cov) -------------------------- diff --git a/services/config_inspector_service.py b/services/config_inspector_service.py index ab68b0e4b..90c30eac0 100644 --- a/services/config_inspector_service.py +++ b/services/config_inspector_service.py @@ -180,6 +180,13 @@ class ConfigService: description="טיימאאוט התחברות ל-MongoDB (מילישניות)", category="database", ), + "MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS": ConfigDefinition( + key="MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS", + services=("webapp", "bot", "scripts"), + default="30", + description="צינון לפני ניסיון התחברות חוזר ל-MongoDB אחרי כשל (שניות); 0 מכבה", + category="database", + ), "MONGODB_RETRY_WRITES": ConfigDefinition( key="MONGODB_RETRY_WRITES", services=("webapp", "bot", "mcp", "webserver"), diff --git a/tests/test_get_db_publishes_after_connect.py b/tests/test_get_db_publishes_after_connect.py new file mode 100644 index 000000000..acfdd2c71 --- /dev/null +++ b/tests/test_get_db_publishes_after_connect.py @@ -0,0 +1,288 @@ +"""``get_db`` מפרסם את הגלובלים רק כשהחיבור מוכן — ולעולם לא ``None``. + +**למה הקובץ הזה קיים.** ``webapp.app.get_db`` מחזיק שני גלובלים, ``client`` +ו-``db``, ומשתמש ב-``client`` כשומר של המסלול המהיר: ``if client is None``. +מי שרואה את השומר מאותחל מדלג על הנעילה ומחזיר את ``db`` כמו שהוא. הקוד +הישן הציב את השומר **לפני** הערך שהוא שומר עליו, ולכן החזיר ``None`` בשני +תרחישים — שניהם שוחזרו לפני שנכתב התיקון: + +1. **מרוץ.** ``server_info()`` הוא סיבוב רשת שלם. קורא מקביל שנכנס באמצעו + ראה ``client`` מוצב ואת ``db`` עדיין ``None``, וקיבל ``None``. +2. **הרעלה קבועה.** כשל ב-``server_info()`` השאיר את ``client`` מוצב על + לקוח שבור. החריגה נזרקה הלאה, אבל כל קריאה עתידית כבר דילגה על האתחול + והחזירה ``None`` — תקלת רשת חולפת אחת שיתקה את התהליך עד ריסטארט. + +התסמין בפרודקשן אינו חריגה ברורה: נתיבים כמו ``search`` עוטפים את הקריאה +ב-``try/except`` ומסתעפים על ``if db is None`` (``webapp/app.py``), כלומר +המערכת מדווחת "מסד לא זמין" ומדרדרת בשקט בזמן שהמסד עצמו בריא. + +**והחצי השלישי: חלון הצינון.** ההרעלה שלמעלה שימשה גם כמפסק — אחרי הכשל +הראשון כל קריאה חזרה מיד. תיקון ההרעלה לבדו גרר את הקצה הנגדי: כל קריאה +מנסה מחדש ומשלמת ``serverSelectionTimeoutMS`` שלם. נמדד מול כתובת שאינה +נפתרת, 20 קריאות: **5.0 שניות** בקוד הישן, **111.0** בלי המפסק, **5.1** עם +חלון הצינון. בסוויטה זה חצה את ``timeout = 60`` שב-``pytest.ini`` והרג עובד +xdist שלם — ``[gw5] node down: Not properly terminated``, בדיוק 60 שניות +אחרי הפלט האחרון שלו. לכן שני טסטים כאן סופרים **מופעי לקוח**, לא זמן: +זמן הוא מדד רועש ב-CI, ומספר ניסיונות ההתחברות הוא הסיבה עצמה. + +הבדיקות כאן אינן דורשות מונגו אמיתי: הן מחליפות את ``MongoClient`` בדמה +שאפשר להאט או להכשיל בדיוק בנקודה הרלוונטית. +""" + +from __future__ import annotations + +import threading +import time + +import pytest + + +class _FakeDatabase: + def __init__(self, name: str) -> None: + self.name = name + + +class _FakeClient: + """דמה של ``MongoClient`` שאפשר להאט או להכשיל ב-``server_info``.""" + + def __init__(self, *_a, delay: float = 0.0, fail: bool = False, **_k) -> None: + self._delay = delay + self._fail = fail + self.closed = False + + def server_info(self): + if self._delay: + time.sleep(self._delay) + if self._fail: + raise RuntimeError("תקלת רשת מדומה") + return {"version": "8.0.0-fake"} + + def __getitem__(self, name): + return _FakeDatabase(name) + + def close(self): + self.closed = True + + +@pytest.fixture(scope="module", autouse=True) +def _drain_the_background_db_pollers(): + """מנתק את תהליכוני הרקע מ-``webapp.app.get_db`` למשך הקובץ הזה. + + ``webapp/push_api.py`` מריץ ``push-sender`` — תהליכון דמון עם + ``while True`` שקורא ל-``webapp.app.get_db()`` בכל סבב. **הוא הקורא + המקביל האמיתי**, ובזכותו המרוץ שהבדיקות כאן מתארות אינו תיאורטי; אבל + הוא גם מזהם, כי הוא נוגע בדיוק באותם גלובלים. נצפה בפועל: סבב שלו + באמצע בדיקה פרסם ``client`` משלו, והבדיקה ספרה אפס ניסיונות התחברות. + + ההשתקה לבדה אינה מספיקה — סבב שכבר נכנס ל-``_send_due_once`` פתר את + השמות לפני ההחלפה, והוא עדיין בדרכו פנימה. לכן: משתיקים את הסבבים + הבאים, ואז **מנקזים** את זה שבתנועה. הלולאה ישנה לפחות 20 שניות בין + סבבים, ולכן אחרי הניקוז לא נותר אף אחד. + + לא נגענו בקוד הייצור בכוונה: ``webapp/app.py`` מחזיק שומר מוכן לכך + (``_is_webapp_runtime``, שכבר מונע את ה-backup scheduler בתהליך שאינו + webapp), אבל הוספתו ל-``push_api`` הייתה משנה איזה תהליך שולח פוש + בפרודקשן — החלטה שאינה חלק מהתיקון הזה. + """ + import webapp.app as wa + import webapp.push_api as push_api + + saved = push_api._send_due_once + push_api._send_due_once = lambda *a, **k: None + try: + lock = wa.__dict__.setdefault("_DB_INIT_LOCK", threading.Lock()) + deadline = time.monotonic() + 40.0 + while time.monotonic() < deadline: + with lock: # ממתין לניסיון שכבר בתוך הנעילה + pass + before = (wa.client, wa.__dict__.get("_DB_LAST_CONNECT_FAILURE_AT")) + # ``serverSelectionTimeoutMS`` המלא הוא 5 שניות; אם דבר לא זז + # יותר מזה, אין סבב בתנועה. + time.sleep(6.0) + if (wa.client, wa.__dict__.get("_DB_LAST_CONNECT_FAILURE_AT")) == before: + break + yield + finally: + push_api._send_due_once = saved + + +@pytest.fixture +def app_module(monkeypatch): + """מבודד את הגלובלים של ``webapp.app`` ומשחזר אותם בסוף. + + ``monkeypatch.setattr`` מטפל בשחזור, כך שגם בדיקה שנופלת אינה מותירה + את המודול מחובר למסד מדומה עבור שאר החבילה. + """ + import webapp.app as wa + + monkeypatch.setattr(wa, "client", None, raising=False) + monkeypatch.setattr(wa, "db", None, raising=False) + monkeypatch.setattr(wa, "MONGODB_URL", "mongodb://fake-host:27017", raising=False) + monkeypatch.setattr(wa, "DATABASE_NAME", "cktest_get_db_publish", raising=False) + # מצב הצינון הוא גלובל של המודול, ולכן כשל מקובץ אחר היה מדליק את + # המפסק כאן ומחזיר None לפני שהבדיקה הספיקה לרוץ. + monkeypatch.setattr(wa, "_DB_LAST_CONNECT_FAILURE_AT", None, raising=False) + return wa + + +def _expire_the_cooldown(wa, monkeypatch) -> None: + """מקדם את השעון מעבר לחלון הצינון, בלי לחכות בפועל. + + ``_connect_is_cooling_down`` נשען על ``time.monotonic``, ולכן מספיק + להזיז את סימן הכשל אחורה. + """ + if wa._DB_LAST_CONNECT_FAILURE_AT is None: + return + past = wa._DB_LAST_CONNECT_FAILURE_AT - (wa._connect_cooldown_seconds() + 1.0) + monkeypatch.setattr(wa, "_DB_LAST_CONNECT_FAILURE_AT", past, raising=False) + + +def test_a_concurrent_caller_never_receives_none(app_module, monkeypatch): + """הקורא השני נכנס בדיוק בזמן סיבוב הרשת של ``server_info``.""" + wa = app_module + created: list[_FakeClient] = [] + + def _factory(*a, **k): + c = _FakeClient(*a, delay=0.5, **k) + created.append(c) + return c + + monkeypatch.setattr(wa, "MongoClient", _factory, raising=True) + + results: dict[str, object] = {} + + def _call(name: str, delay: float) -> None: + time.sleep(delay) + try: + results[name] = wa.get_db() + except BaseException as exc: # noqa: BLE001 — נרשם ומדווח באסרשן + results[name] = exc + + initializer = threading.Thread(target=_call, args=("initializer", 0.0)) + bystander = threading.Thread(target=_call, args=("bystander", 0.25)) + initializer.start() + bystander.start() + initializer.join(timeout=10) + bystander.join(timeout=10) + + assert len(created) == 1, f"נוצרו {len(created)} לקוחות — הנעילה לא החזיקה" + assert isinstance(results.get("initializer"), _FakeDatabase) + assert isinstance(results.get("bystander"), _FakeDatabase), ( + f"קורא מקביל קיבל {results.get('bystander')!r} במקום מסד — " + f"השומר ``client`` פורסם לפני ``db``" + ) + + +def test_a_failed_connection_does_not_poison_the_module(app_module, monkeypatch): + """אחרי כשל חולף, וברגע שחלון הצינון עבר, ההתחברות מנוסה שוב ומצליחה. + + זהו הבאג המקורי: קודם ``client`` נשאר מוצב על לקוח שבור, ולכן שום + קריאה עתידית לא ניסתה שוב — לנצח. הבדיקה מקדמת את השעון במקום לחכות, + כדי שלא תוסיף שניות לסוויטה. + """ + wa = app_module + created: list[_FakeClient] = [] + state = {"fail_next": True} + + def _factory(*a, **k): + c = _FakeClient(*a, fail=state["fail_next"], **k) + state["fail_next"] = False + created.append(c) + return c + + monkeypatch.setattr(wa, "MongoClient", _factory, raising=True) + + with pytest.raises(RuntimeError): + wa.get_db() + + assert wa.client is None, "לקוח שנכשל נשאר מוצב כשומר, וחוסם כל אתחול עתידי" + assert created[0].closed, "הלקוח שלא פורסם לא נסגר — דליפת חיבורים בכל כשל" + + _expire_the_cooldown(wa, monkeypatch) + + assert isinstance(wa.get_db(), _FakeDatabase), ( + "הקריאה שאחרי כשל חולף לא התאוששה גם אחרי שחלון הצינון עבר" + ) + assert len(created) == 2, "לא נוצר לקוח חדש — האתחול דולג" + assert wa._connect_is_cooling_down() is False, ( + "חיבור מוצלח לא ניקה את סימן הכשל, והמפסק היה נדלק שוב לשווא" + ) + + +def test_a_call_inside_the_cooldown_does_not_dial_at_all(app_module, monkeypatch): + """**זו הבדיקה שנופלת על הגרסה שהקפיאה את ה-CI.** + + היא סופרת מופעי ``MongoClient`` ולא זמן. בלי המפסק, הקריאה השנייה בונה + לקוח שני ומשלמת ``serverSelectionTimeoutMS`` שלם — ובסוויטה אמיתית + מספיק שלוש-עשרה כאלה כדי לחצות את ``timeout = 60`` ולהרוג עובד xdist. + """ + wa = app_module + created: list[_FakeClient] = [] + + def _factory(*a, **k): + c = _FakeClient(*a, fail=True, **k) + created.append(c) + return c + + monkeypatch.setattr(wa, "MongoClient", _factory, raising=True) + + with pytest.raises(RuntimeError): + wa.get_db() + assert len(created) == 1 + + for _ in range(12): + wa.get_db() + + assert len(created) == 1, ( + f"נוצרו {len(created)} לקוחות במקום אחד — כל קריאה בתוך חלון הצינון " + f"משלמת timeout מלא, וזה מה שהרג את gw5 ב-CI" + ) + + +def test_a_call_inside_the_cooldown_returns_none_and_does_not_raise(app_module, monkeypatch): + """נעילת חוזה מול 96 אתרי קריאה שאינם עטופים ב-``try``. + + ``None`` הוא חוזה קיים, לא בחירה חדשה: כך התנהג הקוד מאז ומתמיד בכל + קריאה שאחרי הכשל הראשון. מה שהשתנה הוא שהוא זמני ולא נצחי. + """ + wa = app_module + monkeypatch.setattr(wa, "MongoClient", lambda *a, **k: _FakeClient(fail=True), raising=True) + + with pytest.raises(RuntimeError): + wa.get_db() + + # בלי try/except במכוון — חריגה כאן היא בדיוק הכשל שהבדיקה מחפשת + assert wa.get_db() is None + + +def test_the_cooldown_can_be_switched_off(app_module, monkeypatch): + """``MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS=0`` מכבה את המפסק לגמרי. + + בלי הטסט הזה, ערך שאינו נקרא היה נראה כמו מתג עובד. + """ + wa = app_module + monkeypatch.setenv("MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS", "0") + created: list[_FakeClient] = [] + + def _factory(*a, **k): + c = _FakeClient(*a, fail=True, **k) + created.append(c) + return c + + monkeypatch.setattr(wa, "MongoClient", _factory, raising=True) + + for _ in range(2): + with pytest.raises(RuntimeError): + wa.get_db() + + assert len(created) == 2, "המפסק כבוי, ולכן כל קריאה אמורה לנסות מחדש" + + +def test_the_module_is_not_left_connected_to_a_fake(app_module): + """נעילת היקף: הפיקסצ'ר מבודד, כדי שהדמה לא תדלוף לשאר החבילה. + + בלי זה, בדיקה שנופלת באמצע הייתה משאירה את ``webapp.app.db`` מוצב על + ``_FakeDatabase``, וכל קובץ שרץ אחריה היה נכשל מסיבה שאינה קשורה. + """ + assert app_module.client is None + assert app_module.db is None diff --git a/tests/test_share_preview_failure_is_logged.py b/tests/test_share_preview_failure_is_logged.py new file mode 100644 index 000000000..750f2b72e --- /dev/null +++ b/tests/test_share_preview_failure_is_logged.py @@ -0,0 +1,142 @@ +"""כשל בשליפת המטא-דאטה לשיתוף נרשם ללוג, ולא מתחפש ל-404. + +**למה הקובץ הזה קיים.** ה-``except`` בענף ה-``preview`` של ``create_public_share`` +היה ``meta = {}`` בלבד. שתי שורות מתחתיו ``if not meta`` מחזיר **404 "קובץ לא +נמצא"** — כלומר כשל בשאילתה הוגש למשתמש כקובץ שאינו קיים, בלי שום שורת לוג +שתאפשר להבחין בין השניים. זה מה שהחזיק את #3353 מוסתר. + +⚠️ **מה הקובץ הזה מוכיח, ומה לא.** הוא מזריק חריגה מלאכותית מהסטאב, ולכן הוא +ראיה על כך שה-``except`` **מלוגג** — ולא ראיה על כך שמסלול השיתוף עדיין יכול +להישבר. אחרי המעבר ל-``$substrCP`` הסיבה שהפילה אותו בפרודקשן נעלמת, וה-``except`` +נשאר עבור תקלות אחרות (נפילת קישוריות למונגו, ``$split`` על ``code`` שאינו +מחרוזת, ``ObjectId`` פגום). זהו **טסט רגרסיה על הלוגינג בלבד**. הראיה על מסלול +השיתוף עצמו נמצאת ב-``tests/test_snippet_hebrew_offsets_mongo.py``, שמריץ את +הראוט מול מונגו אמיתי. + +הטסטים בודקים את **ההתנהגות** — מה יצא ללוג — ולא את קיום השורה בקוד; קריאת +הקוד הייתה "מאמתת" גם ניסוח שאינו רץ לעולם. זו אותה גישה כמו ב- +``tests/test_search_fallback_is_logged.py``. +""" + +from __future__ import annotations + +import logging + +import pytest +from bson import ObjectId + +FILE_ID = "0123456789abcdef01234567" +USER_ID = 4242 + +_META = { + "file_name": "demo.py", + "programming_language": "python", + "description": "", + "file_size": 8, + "lines_count": 1, + "snippet_preview": "print(1)", +} + + +class _CodeSnippets: + def __init__(self, explode: bool): + self.explode = explode + + def find_one(self, *_a, **_k): + return { + "_id": ObjectId(FILE_ID), + "user_id": USER_ID, + "file_name": "demo.py", + "programming_language": "python", + "description": "", + "code": "print(1)", + } + + def aggregate(self, _pipeline): + if self.explode: + raise RuntimeError("aggregate exploded") + return [dict(_META)] + + +class _InternalShares: + def __init__(self): + self.docs: list = [] + + def create_index(self, *_a, **_k): + return "idx" + + def insert_one(self, doc): + self.docs.append(dict(doc)) + return type("_R", (), {"inserted_id": 1})() + + +class _DB: + def __init__(self, snippets): + self.code_snippets = snippets + self.internal_shares = _InternalShares() + + +@pytest.fixture +def share(monkeypatch): + import webapp.app as wa + + def _post(explode: bool): + monkeypatch.setattr(wa, "get_db", lambda: _DB(_CodeSnippets(explode)), raising=True) + client = wa.app.test_client() + with client.session_transaction() as sess: + sess["user_id"] = USER_ID + sess["user_data"] = {"id": USER_ID, "first_name": "Test"} + return client.post(f"/api/share/{FILE_ID}", json={}) + + return _post + + +def _share_warnings(caplog): + return [r for r in caplog.records if "share preview" in r.getMessage()] + + +def test_a_failing_preview_aggregation_is_logged(share, caplog): + with caplog.at_level(logging.WARNING): + share(explode=True) + + assert _share_warnings(caplog), ( + f"כשל בשאילתת השיתוף חזר שקט: {[r.getMessage() for r in caplog.records]}" + ) + + +def test_the_failure_carries_the_exception_itself(share, caplog): + """בלי ``exc_info`` השורה אומרת "משהו נשבר" ולא **מה**.""" + with caplog.at_level(logging.WARNING): + share(explode=True) + + records = _share_warnings(caplog) + assert records and records[0].exc_info is not None, "השורה אינה נושאת את החריגה" + + +def test_the_404_contract_is_unchanged(share, caplog): + """נעילת היקף: הוספת הלוג לא משנה את מה שהמשתמש מקבל. + + להבחין בין "לא נמצא" ל"השאילתה נשברה" הוא שינוי חוזה API, והוא מכוון + מחוץ להיקף של התיקון הזה. + """ + with caplog.at_level(logging.WARNING): + resp = share(explode=True) + + assert resp.status_code == 404 + # jsonify מקודד עברית ל-\uXXXX, ולכן בודקים את ה-JSON המפורסר ולא את הטקסט הגולמי + assert resp.get_json() == {"ok": False, "error": "קובץ לא נמצא"} + + +def test_a_successful_preview_logs_nothing(share, caplog): + """**האסרשן שהופך את הקובץ למסוגל להיכשל בכיוון הנכון.** + + בלעדיו, ``logger.warning`` שהונח **מחוץ** ל-``except`` היה מספק את שלושת + הטסטים שמעליו — כלומר הם היו עוברים על קוד שמלוגג אזהרה בכל שיתוף מוצלח. + """ + with caplog.at_level(logging.WARNING): + resp = share(explode=False) + + assert resp.status_code == 200, resp.get_data(as_text=True) + assert not _share_warnings(caplog), ( + f"שיתוף מוצלח כתב אזהרה: {[r.getMessage() for r in caplog.records]}" + ) diff --git a/tests/test_snippet_hebrew_offsets_mongo.py b/tests/test_snippet_hebrew_offsets_mongo.py new file mode 100644 index 000000000..73e98f51a --- /dev/null +++ b/tests/test_snippet_hebrew_offsets_mongo.py @@ -0,0 +1,186 @@ +"""#3353 מול מונגו אמיתי — היחידות של חיתוך ה-snippet על טקסט עברי. + +**למה הקובץ הזה קיים, ולמה סטאב לא יכול להחליף אותו.** הבאג הוא בסמנטיקה של +מונגו עצמה: ``$regexFind`` מחזיר ``idx`` בתווים, ``$substrBytes`` מצפה לבייטים, +ובעברית זה או חותך שגוי או **זורק**. אף דמה כתובה-ביד בריפו אינה מדגמנת את זה — +``tests/_fake_mongo.py`` אפילו אין בו ``aggregate``, והוספת אחד פירושה לכתוב +מפרש ביטויי BSON. לכן זה הקובץ היחיד שבאמת מריץ את השאילתה. + +**הוא מדולג ב-CI**, כי הג'וב ``unit-tests`` מגדיר שירות ``mongodb`` בלי ``ports:`` +ואינו רץ בקונטיינר, ולכן שם המארח אינו נפתר. זה מתועד ב-``.github/workflows/ci.yml`` +וב-``tests/conftest.py``. האח שלו שכן רץ בכל PR הוא +``tests/test_snippet_offsets_are_code_points.py``, שבודק את **יחידות** הצינור +בלי להריץ אותו. אף אחד מהשניים אינו מספיק לבדו. + +**כל הערכים שבאסרשנים כאן נמדדו** מול MongoDB 8.0 לפני שנכתבו, ולא חושבו בראש. + +**גישה למסד: תמיד ``wa.get_db()``, לעולם לא ``wa.db``.** הפיקסצ'ר מאפס את +הגלובל ``wa.db`` ל-``None`` (``tests/conftest.py``) כדי לכפות חיבור מחדש למסד +הזמני; ``get_db()`` הוא זה שמאתחל אותו. גישה ישירה ל-``wa.db`` מקבלת ``None`` +ונופלת ב-``AttributeError`` לפני האסרשן הראשון. זה הדפוס בכל שאר הבדיקות +שמשתמשות בפיקסצ'ר. + +הרצה מקומית:: + + NOTE_FONTS_TEST_MONGO_URI='mongodb://127.0.0.1:27017' \\ + pytest tests/test_snippet_hebrew_offsets_mongo.py -v +""" + +from __future__ import annotations + +import logging +from datetime import datetime, timezone + +USER_ID = 987654321 +NEEDLE = "שלום" # ארבעה תווים, שמונה בייטים + + +def _doc(file_name: str, code: str) -> dict: + """מסמך בלי ``file_size`` — כדי שענף ה-``$ifNull`` המחשב באמת ירוץ.""" + now = datetime.now(timezone.utc) + return { + "user_id": USER_ID, + "file_name": file_name, + "code": code, + "programming_language": "text", + "tags": [], + "version": 1, + "is_active": True, + "created_at": now, + "updated_at": now, + } + + +def _search(wa, monkeypatch, query: str): + """קורא ל-``_safe_search`` במסלול ה-regex. + + ``search_type="regex"`` במכוון: הוא נמנע מ-``$text``, שהיה דורש אינדקס + טקסט על המסד הזמני ומפיל את הריצה מסיבה שאינה קשורה לבאג. + """ + monkeypatch.setattr(wa, "search_engine", None, raising=False) + return wa._safe_search(USER_ID, query, limit=50, search_type="regex") + + +# -------------------------------------------------------------------------- +# מסלול החיפוש +# -------------------------------------------------------------------------- + + +def test_a_hebrew_match_is_sliced_and_highlighted_in_characters(wired_mongo, monkeypatch, caplog): + """**כשל שקט — בלי חריגה בכלל.** זה החצי שקל לפספס. + + עם 100 אותיות לפני ההתאמה, ``_snippet_start`` הוא 50, ובייט 50 הוא + **תחילת** תו. לכן ``$substrBytes`` אינו זורק — הוא פשוט מחזיר את בייטים + 50..250, שהם תווים 25..125. נמדד: 100 תווים במקום 154. + + ובמקביל ``_match_len`` היה 8 (בייטים) במקום 4 (תווים), ולכן ההדגשה סימנה + כפול מאורך המילה. + """ + wa = wired_mongo + db = wa.get_db() + code = ("ש" * 100) + NEEDLE + ("ש" * 100) # 204 תווים, 408 בייטים + db.code_snippets.insert_one(_doc("hebrew_silent.txt", code)) + + with caplog.at_level(logging.WARNING): + results = _search(wa, monkeypatch, NEEDLE) + + assert len(results) == 1, "המסמך לא נמצא בכלל" + result = results[0] + + # נמדד: $substrCP(code, 50, 200) מחזיר 154 תווים (204-50), הישן החזיר 100 + assert result.snippet_preview == code[50:250] + assert len(result.snippet_preview) == 154 + + start, end = result.highlight_ranges[0] + assert result.snippet_preview[start:end] == NEEDLE, ( + f"ההדגשה מסמנת {result.snippet_preview[start:end]!r} ולא את המילה עצמה" + ) + assert end - start == 4, "אורך ההדגשה בתווים, לא בבייטים" + + # נעילת היקף: גודל קובץ הוא באמת בייטים + assert result.file_size == len(code.encode("utf-8")) == 408 + + assert not [r for r in caplog.records if "primary aggregation" in r.getMessage()], ( + "המסלול המהיר נפל, כלומר הבדיקה לא בדקה את מה שהיא טוענת" + ) + + +def test_a_hebrew_match_on_an_odd_boundary_does_not_crash_the_pipeline( + wired_mongo, monkeypatch, caplog +): + """**הכשל שמפיל את השאילתה.** + + עם 101 אותיות לפני ההתאמה, ``_snippet_start`` הוא 51 — הבייט השני של תו. + נמדד מול מונגו: ``$substrBytes: Invalid range, starting index is a UTF-8 + continuation byte``. הצינור המהיר מת, והקוד יורד לסריקה מלאה בלי אינדקס. + """ + wa = wired_mongo + db = wa.get_db() + code = ("ש" * 101) + NEEDLE + ("ש" * 100) + db.code_snippets.insert_one(_doc("hebrew_crash.txt", code)) + + with caplog.at_level(logging.WARNING): + results = _search(wa, monkeypatch, NEEDLE) + + # בודקים גם את הלוג ולא רק את התוצאות: הפולבאק הישן עשוי להחזיר משהו + # סביר, ואז אסרשן על התוצאות בלבד היה עובר במקרה. + assert not [r for r in caplog.records if "primary aggregation" in r.getMessage()], ( + "הצינור המהיר נפל על גבול UTF-8 — זה הבאג עצמו" + ) + assert len(results) == 1 + assert results[0].snippet_preview == code[51:251] + + +# -------------------------------------------------------------------------- +# מסלול השיתוף +# -------------------------------------------------------------------------- + + +def _share(wa, file_id): + client = wa.app.test_client() + with client.session_transaction() as sess: + sess["user_id"] = USER_ID + sess["user_data"] = {"id": USER_ID, "first_name": "Test"} + return client.post(f"/api/share/{file_id}", json={}) + + +def test_a_hebrew_file_is_previewed_at_its_full_length(wired_mongo): + """כשל שקט בשיתוף: ה-preview חוזר בשני שלישים מאורכו. + + נמדד: ``$substrBytes(code, 0, 2000)`` על 1500 אותיות עבריות (3000 בייטים) + מחזיר **1000** תווים. בייט 2000 הוא תחילת תו, ולכן אין חריגה — רק חצי + תוכן. + """ + wa = wired_mongo + db = wa.get_db() + code = "ש" * 1500 + inserted = db.code_snippets.insert_one(_doc("hebrew_preview.txt", code)) + + resp = _share(wa, str(inserted.inserted_id)) + assert resp.status_code == 200, resp.get_data(as_text=True) + + share = db.internal_shares.find_one({"share_id": resp.get_json()["share_id"]}) + assert share is not None, "השיתוף לא נשמר" + assert len(share["snippet_preview"]) == 1500 + assert share["file_size"] == len(code.encode("utf-8")) == 3000 + + +def test_a_hebrew_file_is_not_reported_as_missing(wired_mongo): + """**404 שקרי על קובץ שקיים** — הסימפטום הגרוע ביותר ב-#3353. + + נמדד: בייט 2000 של ``"x" + "ש"*1500`` נופל באמצע תו, ומונגו מחזירה + ``ending index is in the middle of a UTF-8 character`` — בדיוק נוסח + השגיאה שהופיע בלוג הפרודקשן. ה-``except`` בלע אותה והתשובה הייתה 404. + """ + wa = wired_mongo + db = wa.get_db() + code = "x" + ("ש" * 1500) + inserted = db.code_snippets.insert_one(_doc("hebrew_404.txt", code)) + + resp = _share(wa, str(inserted.inserted_id)) + + assert resp.status_code == 200, ( + "קובץ קיים דווח כלא נמצא: " + resp.get_data(as_text=True) + ) + share = db.internal_shares.find_one({"share_id": resp.get_json()["share_id"]}) + assert len(share["snippet_preview"]) == 1501 diff --git a/tests/test_snippet_offsets_are_code_points.py b/tests/test_snippet_offsets_are_code_points.py new file mode 100644 index 000000000..fe8bb6413 --- /dev/null +++ b/tests/test_snippet_offsets_are_code_points.py @@ -0,0 +1,280 @@ +"""חיתוך ומדידה של טקסט בצינורות ה-snippet נעשים בתווים, לא בבייטים. + +**למה הקובץ הזה קיים.** ``$regexFind`` מחזיר ``idx`` כאינדקס **תווים** (code +point index, מפורש בתיעוד של מונגו), והקוד הזין אותו ל-``$substrBytes``, שמצפה +ל**בייטים**. באנגלית שתי היחידות זהות ולכן זה עבר סקירה; בעברית כל אות היא שני +בייטים, גבול החיתוך נוחת באמצע תו, ומונגו זורקת ומפילה את כל האגרגציה. בחיפוש +זה הוריד את המערכת לסריקה מלאה בלי אינדקס, ובשיתוף זה החזיר 404 "קובץ לא נמצא" +על קובץ שקיים. + +**מה הקובץ הזה בודק, ומה לא.** הוא מקליט את הצינור ש**נשלח בפועל** ל-``aggregate`` +משני אתרי הקריאה, ובודק עליו תכונת יחידות. הוא אינו מריץ את הצינור, ולכן אינו +יכול להוכיח שמונגו מקבלת אותו ואינו תופס שגיאת off-by-one — לשם כך יש את +``tests/test_snippet_hebrew_offsets_mongo.py``, שמדלג כשאין מונגו זמין. + +מה כן: הקלטה של האובייקט האמיתי שנמסר לדרייבר תופסת תיקון שהוחל על helper שאינו +מחווט, או רק על אחד משני האתרים — שתי אפשרויות שקריאת קוד המקור הייתה "מאמתת". +""" + +from __future__ import annotations + +import pytest +from bson import ObjectId + +BYTE_OPS = {"$strLenBytes", "$substrBytes", "$indexOfBytes"} +CP_OPS = {"$strLenCP", "$substrCP", "$indexOfCP"} + +#: שדות שהמדידה בהם היא על **טקסט**, ולכן חייבת להיות בתווים. +TEXT_FIELDS = ("snippet_preview", "_match_len", "_has_code_match") + +#: ביטויים שנגזרים מ-``$regexFind.idx``, שהוא אינדקס תווים. כל חישוב שנשען +#: עליהם חייב להישאר במרחב התווים. +CODE_POINT_SOURCES = ("$_m.idx", "$_match_idx", "$_snippet_start") + + +def _ops(expr) -> set: + """כל שמות האופרטורים בתת-העץ.""" + found = set() + if isinstance(expr, dict): + for key, value in expr.items(): + if isinstance(key, str) and key.startswith("$"): + found.add(key) + found |= _ops(value) + elif isinstance(expr, (list, tuple)): + for item in expr: + found |= _ops(item) + return found + + +def _fields(pipeline) -> dict: + """שם שדה ← הביטוי שמחשב אותו, מכל שלבי ``$addFields``/``$set``.""" + out = {} + for stage in pipeline or []: + if isinstance(stage, dict): + for key in ("$addFields", "$set"): + inner = stage.get(key) + if isinstance(inner, dict): + out.update(inner) + return out + + +def _mentions(expr, needles) -> bool: + """האם הביטוי מפנה לאחד מהשדות הנתונים (בכל עומק).""" + if isinstance(expr, str): + return expr in needles + if isinstance(expr, dict): + return any(_mentions(v, needles) for v in expr.values()) + if isinstance(expr, (list, tuple)): + return any(_mentions(item, needles) for item in expr) + return False + + +def assert_text_fields_use_code_points(pipeline) -> None: + fields = _fields(pipeline) + for name in TEXT_FIELDS: + if name not in fields: + continue + used = _ops(fields[name]) + offending = used & BYTE_OPS + assert not offending, ( + f"השדה {name!r} מודד טקסט ב{sorted(offending)} — יחידת בייטים. " + f"בעברית זה חותך באמצע אות ומפיל את השאילתה." + ) + assert used & CP_OPS, ( + f"השדה {name!r} אמור להימדד בתווים, ואין בו אף אופרטור מ-{sorted(CP_OPS)}: " + f"{fields[name]!r}" + ) + + +def assert_code_point_indices_never_meet_byte_operators(pipeline) -> None: + """התכונה הכללית — תופסת גם את השדה הבא שמישהו יוסיף.""" + for name, expr in _fields(pipeline).items(): + if not _mentions(expr, CODE_POINT_SOURCES): + continue + offending = _ops(expr) & BYTE_OPS + assert not offending, ( + f"השדה {name!r} נגזר מאינדקס תווים ובכל זאת משתמש ב{sorted(offending)}. " + f"$regexFind.idx הוא code point index — ערבוב היחידות הוא הבאג עצמו." + ) + + +def assert_file_size_is_still_bytes(pipeline) -> None: + """נעילת היקף: ``file_size`` הוא גודל אחסון, ובייטים הם היחידה הנכונה בו. + + בלי האסרשן הזה, "הרחבה" של התיקון ל-``$strLenCP`` הייתה מדווחת קבצים + עבריים בחצי גודלם — בשקט. + """ + expr = _fields(pipeline).get("file_size") + if expr is None: + return + assert "$strLenBytes" in _ops(expr), ( + f"file_size חייב להישאר בבייטים; התקבל {expr!r}" + ) + + +def check_pipeline(pipeline) -> None: + assert_text_fields_use_code_points(pipeline) + assert_code_point_indices_never_meet_byte_operators(pipeline) + assert_file_size_is_still_bytes(pipeline) + + +# -------------------------------------------------------------------------- +# מסלול החיפוש +# -------------------------------------------------------------------------- + + +class _CapturingCodeSnippets: + """מקליט כל צינור שנשלח, ויכול להיכשל במספר הקריאות הראשונות.""" + + def __init__(self, fail_times: int = 0) -> None: + self.pipelines: list = [] + self.fail_times = fail_times + self.calls = 0 + + def aggregate(self, pipeline, **_kwargs): + self.calls += 1 + self.pipelines.append(list(pipeline or [])) + if self.calls <= self.fail_times: + raise RuntimeError(f"forced failure (call {self.calls})") + return [] + + +class _FakeDB: + def __init__(self, collection): + self.code_snippets = collection + + +def _run_search(monkeypatch, fail_times: int = 0): + import webapp.app as wa + + collection = _CapturingCodeSnippets(fail_times=fail_times) + # בלי זה מנוע החיפוש עונה קודם והצינור לא נבנה כלל. + monkeypatch.setattr(wa, "search_engine", None, raising=False) + monkeypatch.setattr(wa, "get_db", lambda: _FakeDB(collection), raising=True) + wa._safe_search(6865105071, "שלום", limit=50) + assert collection.pipelines, "לא נלכד אף צינור — הסטאב לא חובר" + return collection + + +def test_the_search_pipeline_measures_text_in_code_points(monkeypatch): + for pipeline in _run_search(monkeypatch).pipelines: + check_pipeline(pipeline) + + +def test_the_regex_fallback_sends_the_same_corrected_stages(monkeypatch): + """``pipeline2 = list(pipeline)`` הוא העתק רדוד שחולק את אותם שלבים. + + כלומר תיקון אחד מכסה את שתי ההרצות. הטסט קיים כדי שהעובדה הזו תישבר + ברעש אם מישהו יבנה את הפולבאק מחדש במקום להעתיק. + """ + collection = _run_search(monkeypatch, fail_times=1) + assert collection.calls >= 2, "הפולבאק לא רץ, אז אין מה לבדוק" + for pipeline in collection.pipelines: + check_pipeline(pipeline) + + +# -------------------------------------------------------------------------- +# מסלול השיתוף +# -------------------------------------------------------------------------- + +FILE_ID = "0123456789abcdef01234567" +USER_ID = 4242 + + +class _ShareCodeSnippets: + def __init__(self): + self.pipelines: list = [] + + def find_one(self, *_a, **_k): + return { + "_id": ObjectId(FILE_ID), + "user_id": USER_ID, + "file_name": "demo.py", + "programming_language": "python", + "description": "", + "code": "print(1)", + } + + def aggregate(self, pipeline): + self.pipelines.append(list(pipeline or [])) + return [{ + "file_name": "demo.py", + "programming_language": "python", + "description": "", + "file_size": 8, + "lines_count": 1, + "snippet_preview": "print(1)", + }] + + +class _InternalShares: + def __init__(self): + self.docs: list = [] + + def create_index(self, *_a, **_k): + return "idx" + + def insert_one(self, doc): + self.docs.append(dict(doc)) + return type("_R", (), {"inserted_id": 1})() + + +class _ShareDB: + def __init__(self, snippets): + self.code_snippets = snippets + self.internal_shares = _InternalShares() + + +def test_the_share_preview_pipeline_measures_text_in_code_points(monkeypatch): + import webapp.app as wa + + snippets = _ShareCodeSnippets() + monkeypatch.setattr(wa, "get_db", lambda: _ShareDB(snippets), raising=True) + + client = wa.app.test_client() + with client.session_transaction() as sess: + sess["user_id"] = USER_ID + sess["user_data"] = {"id": USER_ID, "first_name": "Test"} + resp = client.post(f"/api/share/{FILE_ID}", json={}) + + # הראוט כולו עטוף ב-try/except → 500. סטטוס מפורש כדי שסטאב חסר יהיה + # רועש ולא ייראה כמו ההתנהגות הנבדקת. + assert resp.status_code == 200, resp.get_data(as_text=True) + assert snippets.pipelines, "לא נלכד אף צינור בשיתוף" + for pipeline in snippets.pipelines: + check_pipeline(pipeline) + + +# -------------------------------------------------------------------------- +# הבודק עצמו +# -------------------------------------------------------------------------- + + +def test_the_checker_rejects_the_pipeline_that_shipped_the_bug(): + """בלי זה אפשר לרוקן את הבודק והסוויטה תישאר ירוקה.""" + buggy = [ + {"$addFields": {"_m": {"$regexFind": {"input": "$code", "regex": "x"}}}}, + {"$addFields": { + "_has_code_match": {"$gt": [{"$strLenBytes": {"$ifNull": ["$_m.match", ""]}}, 0]}, + "_match_idx": {"$ifNull": ["$_m.idx", 0]}, + "_match_len": {"$strLenBytes": {"$ifNull": ["$_m.match", ""]}}, + }}, + {"$addFields": {"_snippet_start": {"$max": [0, {"$subtract": ["$_match_idx", 50]}]}}}, + {"$addFields": { + "snippet_preview": {"$substrBytes": ["$code", "$_snippet_start", 200]}, + "file_size": {"$ifNull": ["$file_size", {"$strLenBytes": "$code"}]}, + }}, + ] + with pytest.raises(AssertionError): + assert_text_fields_use_code_points(buggy) + with pytest.raises(AssertionError): + assert_code_point_indices_never_meet_byte_operators(buggy) + # ונעילת ההיקף דווקא **עוברת** על אותו צינור — היא לא אמורה להשתנות + assert_file_size_is_still_bytes(buggy) + + +def test_the_scope_pin_rejects_a_widened_fix(): + """מי שיחליף גם את ``file_size`` יידע על כך מיד.""" + widened = [{"$addFields": {"file_size": {"$ifNull": ["$file_size", {"$strLenCP": "$code"}]}}}] + with pytest.raises(AssertionError): + assert_file_size_is_still_bytes(widened) diff --git a/webapp/app.py b/webapp/app.py index c1df00aec..28bf7a170 100644 --- a/webapp/app.py +++ b/webapp/app.py @@ -2638,16 +2638,83 @@ def get_pygments_style(theme_name: str) -> str: return 'default' +def _connect_cooldown_seconds() -> float: + """כמה זמן לא מנסים להתחבר שוב אחרי כשל. נקרא בכל פעם, כדי שאפשר יהיה + לכוון בזמן ריצה בלי לטעון מחדש את המודול.""" + try: + raw = str(os.getenv("MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS", "30")).strip() + return max(0.0, float(raw)) if raw else 30.0 + except Exception: + return 30.0 + + +def _connect_is_cooling_down() -> bool: + at = globals().get("_DB_LAST_CONNECT_FAILURE_AT") + if at is None: + return False + cooldown = _connect_cooldown_seconds() + if cooldown <= 0: + return False + # שעון מונוטוני: שינוי שעון מערכת לא יאריך או יקצר את החלון. + return (_time.monotonic() - float(at)) < cooldown + + +def _note_connect_failure() -> None: + globals()["_DB_LAST_CONNECT_FAILURE_AT"] = _time.monotonic() + + +def _note_connect_success() -> None: + globals()["_DB_LAST_CONNECT_FAILURE_AT"] = None + + def get_db(): - """מחזיר חיבור למסד הנתונים""" + """מחזיר חיבור למסד הנתונים. + + **סדר הפרסום של הגלובלים הוא חלק מהחוזה, לא סגנון.** ``client`` הוא + השומר של המסלול המהיר (``if client is None``) — מי שרואה אותו מאותחל + מדלג על הנעילה ומחזיר את ``db`` כמו שהוא. לכן החיבור נבנה במשתנים + מקומיים, ושני הגלובלים נכתבים רק כשהוא מוכן, כשהשומר אחרון. + + בלי זה נפתחו שני חורים, ושניהם החזירו ``None`` במקום מסד: + + 1. **מרוץ.** ``server_info()`` הוא סיבוב רשת שלם. קורא מקביל שנכנס + באמצעו ראה ``client`` כבר מוצב ואת ``db`` עדיין ``None``. + 2. **הרעלה קבועה.** אם ``server_info()`` נכשל, הגלובל ``client`` נשאר + מוצב על לקוח שבור. ה-``except`` זרק הלאה, אבל כל קריאה עתידית כבר + דילגה על האתחול והחזירה ``None`` — כלומר תקלת רשת חולפת אחת שיתקה + את התהליך עד ריסטארט. + + **חלון הצינון, ולמה הוא לא קישוט.** ההרעלה שמעל הייתה גם מפסק: אחרי + הכשל הראשון כל קריאה חזרה מיד, בלי לנסות להתחבר. הבעיה בה מעולם לא + הייתה המהירות אלא ש**היא לנצח**. הסרתה לבדה גררה את הקצה השני — כל + קריאה מנסה מחדש ומשלמת ``serverSelectionTimeoutMS`` שלם. נמדד מול + כתובת שאינה נפתרת, 20 קריאות: **5 שניות** בקוד הישן מול **111** בלעדיו. + בסוויטה זה חצה את ``timeout = 60`` שב-``pytest.ini``, ו-``timeout_method + = thread`` הרג עובד xdist שלם. + + לכן הכשל נרשם עם **זמן**, ובתוך ``MONGODB_CONNECT_RETRY_COOLDOWN_SECONDS`` + (ברירת מחדל 30) הקריאה חוזרת מיד עם ``None`` — בדיוק כמו הקוד הישן. אחרי + שהחלון עובר, ההתחברות מנוסה שוב באמת. מפסק עם זמן פתיחה **סופי**. + + ⚠️ ערך ההחזרה ``None`` הוא חוזה קיים, לא חדש: מתוך 211 אתרי הקריאה + בקוד הייצור, 96 אינם עטופים ב-``try``. שינויו ל"זורק תמיד" הוא ריפקטור + נפרד, ואינו חלק מהתיקון הזה. + """ global client, db # Proper double-checked locking: perform initialization under the lock if client is None: + if _connect_is_cooling_down(): + # כשל טרי: חוזרים מיד, בלי לשלם עוד timeout של בחירת שרת. + return None _db_lock = globals().setdefault("_DB_INIT_LOCK", threading.Lock()) with _db_lock: if client is None: + if _connect_is_cooling_down(): + # נכשל בזמן שחיכינו לנעילה — לא מנסים שוב מיד. + return None if not MONGODB_URL: raise Exception("MONGODB_URL is not configured") + _new_client = None try: # חשוב: ב-ENV יש MONGODB_SERVER_SELECTION_TIMEOUT_MS, אבל בעבר לא חיווטנו אותו לכאן # ולכן בפועל נשאר timeout קשיח של 5 שניות. @@ -2658,16 +2725,21 @@ def get_db(): _server_selection_timeout_ms = 5000 # החזר אובייקטי זמן tz-aware כדי למנוע השוואות naive/aware _t0 = _time.perf_counter() - client = MongoClient( + _new_client = MongoClient( MONGODB_URL, serverSelectionTimeoutMS=_server_selection_timeout_ms, tz_aware=True, tzinfo=timezone.utc, ) # בדיקת חיבור - client.server_info() + _new_client.server_info() duration = max(0.0, float(_time.perf_counter() - _t0)) - db = client[DATABASE_NAME] + _new_db = _new_client[DATABASE_NAME] + # פרסום: קודם הערך, ורק אחריו השומר שמגן עליו + db = _new_db + client = _new_client + _new_client = None + _note_connect_success() try: record_dependency_init("mongodb", duration) except Exception: @@ -2677,6 +2749,14 @@ def get_db(): except Exception: pass except Exception: + # הלקוח לא פורסם, ולכן איש לא יחזיק בו — סוגרים אותו כאן + # כדי שכשל חוזר לא ידלוף חיבורים ו-monitor threads. + if _new_client is not None: + try: + _new_client.close() + except Exception: + pass + _note_connect_failure() logger.exception("Failed to connect to MongoDB") raise # מחוץ לנעילה: הבטח אינדקסים פעם אחת, ללא קריאה חוזרת ל-get_db @@ -2922,7 +3002,7 @@ def _safe_dt_from_doc(value) -> datetime: def _file_last_modified(doc: Dict[str, Any]) -> datetime: """מתי הייצוג שהעמוד מגיש השתנה לאחרונה. - ‏``updated_at`` לבדו אינו מספיק: הוא מציין מתי **התוכן** נערך, ואילו + ``updated_at`` לבדו אינו מספיק: הוא מציין מתי **התוכן** נערך, ואילו העמוד מרנדר גם את מצב המועדף והנעיצה. פעולות המטא-דאטה האלה אינן נוגעות ב-``updated_at`` (ראו ``docs/database/detailed-schema.rst``), ולכן בלי השדות שלהן דפדפן ששולח רק ``If-Modified-Since`` היה מקבל 304 @@ -5141,7 +5221,7 @@ def api_profiler_slow_queries(): # "טבלה ריקה בלי סיבה". ``limit`` לעומתו נשאר סלחני כפי שהיה: ערך פסול שם # מחזיר 50 שורות במקום 20, ולא שורות אחרות. # - # ‏**‏``?min_time=`` ריק פירושו "לא נשלח", ולא "ערך פסול".** מחרוזת ריקה + # **``?min_time=`` ריק פירושו "לא נשלח", ולא "ערך פסול".** מחרוזת ריקה # אינה מספר שגוי — היא היעדר ערך, וממשק שבונה query string משדה ריק שולח # בדיוק את זה. זו גם המוסכמת בשני המקומות האחרים שמטפלים בפרמטר: ב-``main`` # (``float(min_time) if min_time else None``) ובראוט של הבוט @@ -9479,9 +9559,18 @@ def _safe_search(user_id: int, query: str, **kwargs): '_m': {'$regexFind': {'input': '$code', 'regex': pattern, 'options': 'i'}}, }}, {'$addFields': { - '_has_code_match': {'$gt': [{'$strLenBytes': {'$ifNull': ['$_m.match', '']}}, 0]}, + # $strLenCP ולא $strLenBytes: כל המדידות כאן הן על טקסט. + # $regexFind מחזיר את idx כאינדקס **תווים** (code point index, + # מפורש בתיעוד של מונגו), ולכן כל מה שנגזר ממנו חייב להישאר + # במרחב התווים. ערבוב היחידות הוא #3353. + '_has_code_match': {'$gt': [{'$strLenCP': {'$ifNull': ['$_m.match', '']}}, 0]}, '_match_idx': {'$ifNull': ['$_m.idx', 0]}, - '_match_len': {'$strLenBytes': {'$ifNull': ['$_m.match', '']}}, + # _match_len נכנס ל-highlight_ranges יחד עם _match_idx שהוא תווים. + # הצרכן הוא highlightSnippet ב-webapp/static/js/global_search.js, + # שחותך ב-text.slice() — יחידות UTF-16. בעברית זה שקול ל-code + # points כי כל האותיות בטווח ה-BMP; ⚠️ אמוג'י בקוד ישבור את + # השקילות הזו (תו אחד = שתי יחידות UTF-16). + '_match_len': {'$strLenCP': {'$ifNull': ['$_m.match', '']}}, }}, {'$addFields': { # אם אין התאמה בקוד (למשל התאמה הייתה בשם קובץ/תיאור/תגיות דרך $text), @@ -9495,8 +9584,12 @@ def _safe_search(user_id: int, query: str, **kwargs): }, }}, {'$addFields': { - 'snippet_preview': {'$substrBytes': ['$code', '$_snippet_start', 200]}, + # $substrCP ולא $substrBytes: _snippet_start הוא אינדקס תווים. + # $substrBytes קיבל אותו כאילו היה בייטים, ובעברית הגבול נחת + # באמצע אות — מונגו זרקה והפילה את כל האגרגציה (#3353). + 'snippet_preview': {'$substrCP': ['$code', '$_snippet_start', 200]}, # מטא-דאטה קל (למסמכים חדשים נשמר כבר; למסמכים ישנים מחשבים בריצה) + # ⚠️ file_size נשאר בבייטים במכוון — זה גודל אחסון, לא אורך טקסט. 'file_size': {'$ifNull': ['$file_size', {'$strLenBytes': '$code'}]}, 'lines_count': {'$ifNull': ['$lines_count', {'$size': {'$split': ['$code', '\n']}}]}, # highlight range יחיד (יחסי ל-snippet) עבור התאמה הראשונה, רק אם באמת נמצאה התאמה בקוד @@ -9567,7 +9660,7 @@ def _safe_search(user_id: int, query: str, **kwargs): # ביותר שנרשמה (3,730ms), והיא רצה רק מפני שהמסלול המהיר נפל. # # ההערה שהייתה כאן ניחשה "למשל אין אינדקס טקסט", והניחוש הזה **נבדק - # ונפסל**: ‏``search_text_idx`` קיים, ואותה שאילתת ``$text`` בדיוק רצה + # ונפסל**: ``search_text_idx`` קיים, ואותה שאילתת ``$text`` בדיוק רצה # מול הקלאסטר ומחזירה תוצאות. הסיבה האמיתית הייתה בלתי נראית, וזה מה # שהשורה הבאה מתקנת. # @@ -9642,7 +9735,7 @@ def _safe_search(user_id: int, query: str, **kwargs): ] docs = list(db.code_snippets.aggregate(old_pipeline, allowDiskUse=True)) except Exception: - # ‏**זה המסלול שמחזיר "לא נמצאו תוצאות" על כשל.** בלי לוג, חיפוש + # **זה המסלול שמחזיר "לא נמצאו תוצאות" על כשל.** בלי לוג, חיפוש # שנשבר נראה בדיוק כמו חיפוש שלא מצא כלום — וזה ההבדל היחיד # שחשוב למשתמש. logger.warning( @@ -11056,7 +11149,7 @@ def _timeline_recent_files_query(user_id: int, recent_cutoff: datetime, def _aggregate_snippets(db, pipeline: List[Dict[str, Any]]): - """‏``aggregate`` על ``code_snippets`` עם ``allowDiskUse``, ועם נפילה לאחור. + """``aggregate`` על ``code_snippets`` עם ``allowDiskUse``, ועם נפילה לאחור. ``allowDiskUse`` מיותר בשרת בתצורת ברירת מחדל (``allowDiskUseByDefault`` הוא ``true``) אבל מגן על שרת שהוקשח עם ``false``. הנפילה לאחור על @@ -12120,7 +12213,7 @@ def files(): # הכנת מפתח Cache ייחודי לפרמטרים # - # ‏**הדגל בתחילית ולא בתוך** ``_params``\\ **, וזה לא סגנון.** העמוד הזה + # **הדגל בתחילית ולא בתוך** ``_params``\\ **, וזה לא סגנון.** העמוד הזה # שומר את ה-HTML המרונדר, ושתי התצוגות מייצרות HTML שונה. אילו הדגל היה # רק בתוך ``_params``, ענף ה-``except`` שמתחתיו — שנופל למפתח קבוע אחד — # היה מגיש לשתיהן את אותו HTML. בתחילית שני המסלולים מבדילים. @@ -13069,7 +13162,7 @@ def view_file(file_id): resp.headers['ETag'] = etag resp.headers['Last-Modified'] = last_modified_str return resp - # ‏``If-Modified-Since`` לבדו אינו משמש כאן לוולידציה, ובכוונה. + # ``If-Modified-Since`` לבדו אינו משמש כאן לוולידציה, ובכוונה. # העמוד מרנדר את מצב המועדף והנעיצה לתוך ה-HTML, ואין שדה שמתעד # **מתי המצב הזה השתנה**: ``favorited_at`` אומר מתי סומן, ולכן אחרי # הסרת סימון הוא מתאפס — וה-``Last-Modified`` הנגזר ממנו נסוג אחורה. @@ -16162,9 +16255,14 @@ def create_public_share(file_id): agg = list(db.code_snippets.aggregate([ {'$match': {'_id': ObjectId(file_id), 'user_id': user_id}}, {'$addFields': { + # ⚠️ file_size נשאר בבייטים במכוון — גודל אחסון. 'file_size': {'$ifNull': ['$file_size', {'$strLenBytes': '$code'}]}, 'lines_count': {'$ifNull': ['$lines_count', {'$size': {'$split': ['$code', '\n']}}]}, - 'snippet_preview': {'$substrBytes': ['$code', 0, 2000]}, + # $substrCP: התקרה היא 2000 **תווים**, כמו במסלול + # ה-download שחותך code[:2000] בפייתון. עם $substrBytes + # קובץ עברי חזר בשני שלישים מאורכו, וכשגבול 2000 + # הבייטים נחת באמצע אות — נזרקה חריגה (#3353). + 'snippet_preview': {'$substrCP': ['$code', 0, 2000]}, }}, {'$project': { 'file_name': 1, @@ -16178,6 +16276,17 @@ def create_public_share(file_id): ])) meta = agg[0] if agg and isinstance(agg[0], dict) else {} except Exception: + # ⚠️ הכשל הזה נרשם, ולא נבלע. + # + # meta = {} מוביל ישירות ל-404 "קובץ לא נמצא" שתי שורות + # מכאן — תשובה שנראית בדיוק כמו קובץ שאינו קיים, בזמן + # שהקובץ קיים והשאילתה היא שנשברה. בלי השורה הזו אין שום + # דרך להבחין בין השניים, וזה מה שהחזיק את #3353 מוסתר. + logger.warning( + "share preview: metadata aggregation failed, returning 404", + exc_info=True, + extra={"event": "share_preview_pipeline_failed", "file_id": str(file_id)}, + ) meta = {} if not meta: return jsonify({'ok': False, 'error': 'קובץ לא נמצא'}), 404