fix(collect): handle login target close race
Tests / Python 3.11 / Windows (push) Has been cancelled
Tests / Python 3.11 / Windows (push) Has been cancelled
This commit is contained in:
@@ -192,6 +192,26 @@ class AccountsTests(TempDirMixin, unittest.TestCase):
|
||||
|
||||
self.assert_removed(temp_dir)
|
||||
|
||||
def test_login_status_text_marks_target_failure_as_temporarily_uncertain(self):
|
||||
self.assertEqual(
|
||||
"登录状态暂不可确认",
|
||||
accounts.login_status_text(
|
||||
{
|
||||
"logged_in": False,
|
||||
"reason": "LOGIN_CHECK_TARGET_UNAVAILABLE",
|
||||
}
|
||||
),
|
||||
)
|
||||
self.assertEqual(
|
||||
"登录状态暂不可确认",
|
||||
accounts.login_status_text(
|
||||
{
|
||||
"logged_in": False,
|
||||
"reason": "LOGIN_CHECK_FAILED: Target closed",
|
||||
}
|
||||
),
|
||||
)
|
||||
|
||||
def test_duplicate_alias_raises_clear_error(self):
|
||||
with self.make_temp_dir() as temp_dir:
|
||||
cfg = self.make_config(temp_dir)
|
||||
|
||||
@@ -35,6 +35,60 @@ class FakeBrowserCDP:
|
||||
|
||||
|
||||
class CdpTests(unittest.TestCase):
|
||||
def test_wait_target_closed_returns_when_target_disappears(self):
|
||||
targets = [
|
||||
[{"id": "target-closing", "type": "page"}],
|
||||
[],
|
||||
]
|
||||
with mock.patch("app.cdp.http_get", side_effect=targets) as http_get, mock.patch(
|
||||
"app.cdp.time.sleep"
|
||||
) as sleep:
|
||||
closed = cdp.wait_target_closed(
|
||||
"target-closing",
|
||||
host="127.0.0.1:9222",
|
||||
timeout=1.0,
|
||||
poll_interval=0.1,
|
||||
)
|
||||
|
||||
self.assertTrue(closed)
|
||||
sleep.assert_called_once_with(0.1)
|
||||
self.assertLessEqual(http_get.call_args_list[0].kwargs["timeout"], 1.0)
|
||||
|
||||
def test_wait_target_closed_stops_at_timeout(self):
|
||||
with mock.patch(
|
||||
"app.cdp.http_get",
|
||||
return_value=[{"id": "target-closing", "type": "page"}],
|
||||
) as http_get, mock.patch("app.cdp.time.sleep") as sleep:
|
||||
closed = cdp.wait_target_closed(
|
||||
"target-closing",
|
||||
host="127.0.0.1:9222",
|
||||
timeout=0,
|
||||
)
|
||||
|
||||
self.assertFalse(closed)
|
||||
sleep.assert_not_called()
|
||||
self.assertEqual(0.05, http_get.call_args.kwargs["timeout"])
|
||||
|
||||
def test_close_tab_and_wait_only_waits_after_close_request_succeeds(self):
|
||||
with mock.patch("app.cdp.close_tab", return_value=True) as close_tab, mock.patch(
|
||||
"app.cdp.wait_target_closed",
|
||||
return_value=True,
|
||||
) as wait_target_closed:
|
||||
closed = cdp.close_tab_and_wait(
|
||||
"target-closing",
|
||||
host="127.0.0.1:9222",
|
||||
timeout=1.5,
|
||||
)
|
||||
|
||||
self.assertTrue(closed)
|
||||
close_tab.assert_called_once_with("target-closing", host="127.0.0.1:9222")
|
||||
wait_target_closed.assert_called_once_with(
|
||||
"target-closing",
|
||||
host="127.0.0.1:9222",
|
||||
timeout=1.5,
|
||||
poll_interval=0.1,
|
||||
)
|
||||
|
||||
def test_create_tab_info_can_request_background_target(self):
|
||||
FakeBrowserCDP.sent = []
|
||||
FakeBrowserCDP.fail_first_create = False
|
||||
|
||||
+138
-5
@@ -382,6 +382,104 @@ class EditorLoginTests(unittest.TestCase):
|
||||
self.assertIn("SPC_ST", status["cookie_names"])
|
||||
self.assertTrue(FakeCDP.instances[0].closed)
|
||||
|
||||
def test_login_status_reselects_live_target_after_closing_target_fails(self):
|
||||
pages = [
|
||||
{
|
||||
"id": "target-closing",
|
||||
"type": "page",
|
||||
"url": "https://seller.shopee.tw/portal/product/48663456321",
|
||||
"webSocketDebuggerUrl": "ws-closing",
|
||||
},
|
||||
{
|
||||
"id": "target-live",
|
||||
"type": "page",
|
||||
"url": "https://seller.shopee.tw/portal/product/52313423890",
|
||||
"webSocketDebuggerUrl": "ws-live",
|
||||
},
|
||||
]
|
||||
|
||||
def cdp_factory(ws):
|
||||
instance = FakeCDP(
|
||||
ws,
|
||||
url=next(page["url"] for page in pages if page["webSocketDebuggerUrl"] == ws),
|
||||
cookies=[cookie("SPC_ST")] if ws == "ws-live" else [],
|
||||
)
|
||||
if ws == "ws-closing":
|
||||
original_send = instance.send
|
||||
|
||||
def send(method, params=None):
|
||||
if method == "Network.getAllCookies":
|
||||
raise RuntimeError("Target closed")
|
||||
return original_send(method, params)
|
||||
|
||||
instance.send = send
|
||||
return instance
|
||||
|
||||
with mock.patch("app.editor.http_get", return_value=pages), mock.patch(
|
||||
"app.editor.CDP",
|
||||
side_effect=cdp_factory,
|
||||
), mock.patch("app.editor.time.sleep"):
|
||||
status = editor.login_status({"debug_port": 9222}, timeout=1)
|
||||
|
||||
self.assertTrue(status["logged_in"])
|
||||
self.assertIsNone(status["reason"])
|
||||
self.assertEqual(2, status["probe_attempts"])
|
||||
self.assertEqual(["ws-closing", "ws-live"], [item.ws for item in FakeCDP.instances])
|
||||
self.assertTrue(all(item.closed for item in FakeCDP.instances))
|
||||
|
||||
def test_login_status_target_failure_is_not_no_session_cookie(self):
|
||||
page = {
|
||||
"id": "target-closing",
|
||||
"type": "page",
|
||||
"url": "https://seller.shopee.tw/portal/product/48663456321",
|
||||
"webSocketDebuggerUrl": "ws-closing",
|
||||
}
|
||||
|
||||
def cdp_factory(ws):
|
||||
instance = FakeCDP(ws, url=page["url"])
|
||||
|
||||
def send(method, params=None):
|
||||
if method == "Network.getAllCookies":
|
||||
raise RuntimeError("Target closed")
|
||||
return {}
|
||||
|
||||
instance.send = send
|
||||
return instance
|
||||
|
||||
with mock.patch("app.editor.http_get", return_value=[page]), mock.patch(
|
||||
"app.editor.CDP",
|
||||
side_effect=cdp_factory,
|
||||
):
|
||||
status = editor.login_status({"debug_port": 9222}, timeout=0)
|
||||
|
||||
self.assertFalse(status["logged_in"])
|
||||
self.assertEqual("LOGIN_CHECK_TARGET_UNAVAILABLE", status["reason"])
|
||||
self.assertFalse(status["cookie_read_succeeded"])
|
||||
self.assertNotEqual("NO_SESSION_COOKIE", status["reason"])
|
||||
|
||||
def test_login_status_url_probe_failure_is_not_no_session_cookie(self):
|
||||
page = {
|
||||
"id": "target-closing",
|
||||
"type": "page",
|
||||
"url": "https://seller.shopee.tw/portal/product/48663456321",
|
||||
"webSocketDebuggerUrl": "ws-closing",
|
||||
}
|
||||
instance = FakeCDP("ws-closing", url=page["url"], cookies=[])
|
||||
|
||||
def fail_url_probe(_expression):
|
||||
raise RuntimeError("Target closed")
|
||||
|
||||
instance.val = fail_url_probe
|
||||
with mock.patch("app.editor.http_get", return_value=[page]), mock.patch(
|
||||
"app.editor.CDP",
|
||||
return_value=instance,
|
||||
):
|
||||
status = editor.login_status({"debug_port": 9222}, timeout=0)
|
||||
|
||||
self.assertFalse(status["logged_in"])
|
||||
self.assertEqual("LOGIN_CHECK_TARGET_UNAVAILABLE", status["reason"])
|
||||
self.assertFalse(status["cookie_read_succeeded"])
|
||||
|
||||
def test_login_status_false_without_session_cookie(self):
|
||||
with mock.patch(
|
||||
"app.editor.http_get",
|
||||
@@ -604,6 +702,35 @@ class EditorLoginTests(unittest.TestCase):
|
||||
close_tab.assert_not_called()
|
||||
create_tab_info.assert_not_called()
|
||||
|
||||
def test_open_product_background_failure_confirms_created_target_closed(self):
|
||||
fake = FakeProductCDP(
|
||||
"ws-new",
|
||||
ready=False,
|
||||
toasts=[{"text": "please input correct product id", "visible": False}],
|
||||
)
|
||||
with mock.patch("app.editor.find_product_tab", return_value=None), mock.patch(
|
||||
"app.editor.create_tab_info",
|
||||
return_value={"id": "target-new", "webSocketDebuggerUrl": "ws-new"},
|
||||
), mock.patch("app.editor.CDP", return_value=fake), mock.patch(
|
||||
"app.editor._ensure_page_domains"
|
||||
), mock.patch(
|
||||
"app.editor.close_tab_and_wait",
|
||||
return_value=True,
|
||||
) as close_tab_and_wait:
|
||||
with self.assertRaises(editor.EditorError):
|
||||
editor.open_product(
|
||||
{"debug_port": 9223},
|
||||
"bad-item",
|
||||
bring_to_front=False,
|
||||
)
|
||||
|
||||
self.assertTrue(fake.closed)
|
||||
close_tab_and_wait.assert_called_once_with(
|
||||
"target-new",
|
||||
host="127.0.0.1:9223",
|
||||
timeout=2.0,
|
||||
)
|
||||
|
||||
def test_collect_closes_only_auto_created_product_tab(self):
|
||||
cdp = FakeProductCDP("ws-new")
|
||||
cdp.target_id = "target-new"
|
||||
@@ -619,7 +746,7 @@ class EditorLoginTests(unittest.TestCase):
|
||||
), mock.patch(
|
||||
"app.editor.download_cover",
|
||||
return_value="images/main/51100639510_old.jpg",
|
||||
), mock.patch("app.editor.close_tab", return_value=True) as close_tab:
|
||||
), mock.patch("app.editor.close_tab_and_wait", return_value=True) as close_tab_and_wait:
|
||||
result = editor.collect(
|
||||
{"debug_port": 9222},
|
||||
{"item_id": "51100639510", "old_cover_path": "old.jpg"},
|
||||
@@ -633,7 +760,12 @@ class EditorLoginTests(unittest.TestCase):
|
||||
bring_to_front=False,
|
||||
)
|
||||
self.assertTrue(cdp.closed)
|
||||
close_tab.assert_called_once_with("target-new", host="127.0.0.1:9222")
|
||||
self.assertTrue(result["close_target_confirmed"])
|
||||
close_tab_and_wait.assert_called_once_with(
|
||||
"target-new",
|
||||
host="127.0.0.1:9222",
|
||||
timeout=2.0,
|
||||
)
|
||||
|
||||
def test_collect_keeps_reused_product_tab_open(self):
|
||||
cdp = FakeProductCDP("ws-existing")
|
||||
@@ -650,14 +782,15 @@ class EditorLoginTests(unittest.TestCase):
|
||||
), mock.patch(
|
||||
"app.editor.download_cover",
|
||||
return_value="images/main/51100639510_old.jpg",
|
||||
), mock.patch("app.editor.close_tab") as close_tab:
|
||||
editor.collect(
|
||||
), mock.patch("app.editor.close_tab_and_wait") as close_tab_and_wait:
|
||||
result = editor.collect(
|
||||
{"debug_port": 9222},
|
||||
{"item_id": "51100639510", "old_cover_path": "old.jpg"},
|
||||
)
|
||||
|
||||
self.assertTrue(cdp.closed)
|
||||
close_tab.assert_not_called()
|
||||
self.assertIsNone(result["close_target_confirmed"])
|
||||
close_tab_and_wait.assert_not_called()
|
||||
|
||||
def test_read_product_image_urls_returns_all_images_in_page_order(self):
|
||||
cdp = FakeProductCDP("ws-existing", rects=cover_rects(3, prefix="main"))
|
||||
|
||||
+27
-3
@@ -8465,6 +8465,7 @@ class GuiTests(TempDirMixin, unittest.TestCase):
|
||||
db_path=cfg["db_path"],
|
||||
config=cfg,
|
||||
preflight=False,
|
||||
diagnostic_log_dir=os.path.join(temp_dir, "logs"),
|
||||
).execute()
|
||||
|
||||
self.assertTrue(summary["ok"])
|
||||
@@ -8480,6 +8481,19 @@ class GuiTests(TempDirMixin, unittest.TestCase):
|
||||
self.assertEqual(1, sleep.call_count)
|
||||
self.assertEqual(2, collect.call_count)
|
||||
|
||||
events = db.list_run_log_events(summary["run_id"], path=cfg["db_path"])
|
||||
messages = "\n".join(event.message for event in events)
|
||||
self.assertIn(
|
||||
"step=login_check result=retry detail=任务 "
|
||||
f"{tasks[0].id} 商品 51100639510",
|
||||
messages,
|
||||
)
|
||||
self.assertIn(
|
||||
"step=login_check result=recovered detail=任务 "
|
||||
f"{tasks[0].id} 商品 51100639510",
|
||||
messages,
|
||||
)
|
||||
|
||||
updated = db.list_tasks(batch_id=batch_id, path=cfg["db_path"])
|
||||
self.assertTrue(all(task.stage == "collected" for task in updated))
|
||||
self.assertTrue(all(task.status == "success" for task in updated))
|
||||
@@ -8516,7 +8530,11 @@ class GuiTests(TempDirMixin, unittest.TestCase):
|
||||
with mock.patch("app.gui.accounts.detect_login", return_value=status) as detect_login, \
|
||||
mock.patch(
|
||||
"app.gui.editor.collect",
|
||||
return_value={"old_title": "旧标题", "old_cover_path": "old.jpg"},
|
||||
return_value={
|
||||
"old_title": "旧标题",
|
||||
"old_cover_path": "old.jpg",
|
||||
"close_target_confirmed": False,
|
||||
},
|
||||
) as collect, \
|
||||
mock.patch("app.gui.workers.time.sleep") as sleep:
|
||||
summary = CollectWorker(
|
||||
@@ -8524,6 +8542,7 @@ class GuiTests(TempDirMixin, unittest.TestCase):
|
||||
db_path=cfg["db_path"],
|
||||
config=cfg,
|
||||
preflight=False,
|
||||
diagnostic_log_dir=os.path.join(temp_dir, "logs"),
|
||||
).execute()
|
||||
|
||||
self.assertTrue(summary["ok"])
|
||||
@@ -8544,6 +8563,11 @@ class GuiTests(TempDirMixin, unittest.TestCase):
|
||||
events = db.list_run_log_events(summary["run_id"], path=cfg["db_path"])
|
||||
messages = "\n".join(event.message for event in events)
|
||||
self.assertIn("登录状态检测暂时不稳定", messages)
|
||||
self.assertIn(
|
||||
"step=close_product result=uncertain detail=任务 "
|
||||
f"{tasks[0].id} 商品 51100639510",
|
||||
messages,
|
||||
)
|
||||
self.assertNotIn("采集中途掉登录", messages)
|
||||
|
||||
self.assert_removed(temp_dir)
|
||||
@@ -8604,9 +8628,9 @@ class GuiTests(TempDirMixin, unittest.TestCase):
|
||||
|
||||
events = db.list_run_log_events(summary["run_id"], path=cfg["db_path"])
|
||||
messages = "\n".join(event.message for event in events)
|
||||
self.assertIn("LOGIN_CHECK_FAILED", messages)
|
||||
self.assertIn("登录检测调用失败", messages)
|
||||
self.assertNotIn("LOGIN_CHECK_FAILED", messages)
|
||||
self.assertNotIn("SECRET-TOKEN", messages)
|
||||
self.assertIn("token=***", messages)
|
||||
|
||||
self.assert_removed(temp_dir)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user