Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import axios from "axios";
import { useContext, useState } from "react";
import { useParams } from "react-router-dom";
import useSWR from "swr";
Expand All @@ -9,12 +10,20 @@ export const useFetchActor = (actorId: string | null) => {
return useSWR(
actorId ? ["useActorDetail", actorId] : null,
async ([_, actorId]) => {
const actor_resp = await getActor(actorId);
const data: ActorResp = actor_resp?.data;
const { data: rspData } = data;
try {
const actor_resp = await getActor(actorId);
const data: ActorResp = actor_resp?.data;
const { data: rspData } = data;

if (rspData.detail) {
return rspData.detail;
if (rspData.detail) {
return rspData.detail;
}
Comment thread
chenyuan99 marked this conversation as resolved.
} catch (e) {
// The API returns 404 for an unknown actor ID, which axios rejects on.
if (axios.isAxiosError(e) && e.response?.status === 404) {
return undefined;
}
throw e;
}
},
);
Expand All @@ -28,19 +37,24 @@ export const useActorDetail = () => {
const { data: actorDetail, isLoading } = useSWR(
["useActorDetail", params.actorId],
async ([_, actorId]) => {
const actor_resp = await getActor(actorId);
const data: ActorResp = actor_resp?.data;
const { data: rspData, msg, result } = data;
if (msg) {
setMsg(msg);
}

if (result === false) {
setMsg("Actor Query Error Please Check Actor Id");
}
try {
const actor_resp = await getActor(actorId);
const data: ActorResp = actor_resp?.data;
const { data: rspData, msg } = data;
if (msg) {
setMsg(msg);
}

if (rspData.detail) {
return rspData.detail;
if (rspData.detail) {
return rspData.detail;
}
Comment thread
chenyuan99 marked this conversation as resolved.
} catch (e) {
// The API returns 404 for an unknown actor ID, which axios rejects on.
if (axios.isAxiosError(e) && e.response?.status === 404) {
setMsg("Actor Query Error Please Check Actor Id");
return undefined;
}
throw e;
}
},
{ refreshInterval: API_REFRESH_INTERVAL_MS },
Expand Down
32 changes: 20 additions & 12 deletions python/ray/dashboard/client/src/pages/node/hook/useNodeDetail.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import axios from "axios";
import { useContext, useState } from "react";
import { useParams } from "react-router-dom";
import useSWR from "swr";
Expand All @@ -18,20 +19,27 @@ export const useNodeDetail = () => {
const { data: nodeDetail, isLoading } = useSWR(
["useNodeDetail", params.id],
async ([_, nodeId]) => {
const { data } = await getNodeDetail(nodeId);
const { data: rspData, msg, result } = data;
try {
const { data } = await getNodeDetail(nodeId);
const { data: rspData, msg } = data;

if (msg) {
setMsg(msg);
}

if (result === false) {
setMsg("Node Query Error Please Check Node Name");
setRefresh(false);
}
if (msg) {
setMsg(msg);
}

if (rspData?.detail) {
return rspData.detail;
if (rspData?.detail) {
return rspData.detail;
}
Comment thread
chenyuan99 marked this conversation as resolved.
} catch (e) {
// The API returns 404 for an unknown node ID, which axios rejects on.
// Keep auto-refresh running: a 404 can be transient (e.g. the node is
// not in the dashboard's node table yet), and stopping the refresh
// would leave the page stuck on the error until a manual reload.
if (axios.isAxiosError(e) && e.response?.status === 404) {
setMsg("Node Query Error Please Check Node Name");
return undefined;
Comment thread
cursor[bot] marked this conversation as resolved.
}
throw e;
}
},
{ refreshInterval: isRefreshing ? API_REFRESH_INTERVAL_MS : 0 },
Expand Down
5 changes: 4 additions & 1 deletion python/ray/dashboard/modules/node/datacenter.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,12 @@ def _extract_workers_for_node(cls, node_physical_stats, node_stats):

@classmethod
async def get_node_info(cls, node_id, get_summary=False):
node = DataSource.nodes.get(node_id)
if node is None:
return None

node_physical_stats = dict(DataSource.node_physical_stats.get(node_id, {}))
node_stats = dict(DataSource.node_stats.get(node_id, {}))
node = DataSource.nodes.get(node_id, {})

if get_summary:
node_physical_stats.pop("workers", None)
Expand Down
16 changes: 14 additions & 2 deletions python/ray/dashboard/modules/node/node_head.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ async def get_all_nodes(self, req) -> aiohttp.web.Response:
)
else:
return dashboard_optional_utils.rest_response(
status_code=dashboard_utils.HTTPStatusCode.INTERNAL_ERROR,
status_code=dashboard_utils.HTTPStatusCode.BAD_REQUEST,
Comment thread
chenyuan99 marked this conversation as resolved.
message=f"Unknown view {view}",
)

Expand All @@ -422,6 +422,11 @@ async def get_all_nodes(self, req) -> aiohttp.web.Response:
async def get_node(self, req) -> aiohttp.web.Response:
node_id = req.match_info.get("node_id")
node_info = await DataOrganizer.get_node_info(node_id)
if node_info is None:
return dashboard_optional_utils.rest_response(
status_code=dashboard_utils.HTTPStatusCode.NOT_FOUND,
message=f"Node {node_id} not found.",
)
return dashboard_optional_utils.rest_response(
status_code=dashboard_utils.HTTPStatusCode.OK,
message="Node details fetched.",
Expand Down Expand Up @@ -718,10 +723,17 @@ async def get_all_actors(self, req) -> aiohttp.web.Response:
async def get_actor(self, req) -> aiohttp.web.Response:
actor_id = req.match_info.get("actor_id")
actors = await DataOrganizer.get_actor_infos(actor_ids=[actor_id])
# `get_actor_infos` returns a `None` entry for unknown actor IDs.
actor_detail = actors.get(actor_id)
if actor_detail is None:
Comment thread
chenyuan99 marked this conversation as resolved.
return dashboard_optional_utils.rest_response(
status_code=dashboard_utils.HTTPStatusCode.NOT_FOUND,
message=f"Actor {actor_id} not found.",
)
return dashboard_optional_utils.rest_response(
status_code=dashboard_utils.HTTPStatusCode.OK,
message="Actor details fetched.",
detail=actors[actor_id],
detail=actor_detail,
)

@routes.get("/test/dump")
Expand Down
12 changes: 12 additions & 0 deletions python/ray/dashboard/modules/node/tests/test_actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,5 +374,17 @@ class InfeasibleActor:
raise Exception(f"Timed out while testing, {ex_stack}")


def test_actor_not_found_status_code(disable_aiohttp_cache, ray_start_with_dashboard):
"""`GET /logical/actors/{actor_id}` returns 404 for an unknown actor ID."""
assert wait_until_server_available(ray_start_with_dashboard["webui_url"]) is True
webui_url = format_web_url(ray_start_with_dashboard["webui_url"])

resp = requests.get(f"{webui_url}/logical/actors/{'f' * 32}")
assert resp.status_code == 404, resp.text
resp_json = resp.json()
assert resp_json["result"] is False
assert "not found" in resp_json["msg"]


if __name__ == "__main__":
sys.exit(pytest.main(["-v", __file__]))
46 changes: 46 additions & 0 deletions python/ray/dashboard/modules/node/tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,5 +412,51 @@ def _check_worker_pids():
wait_for_condition(_check_worker_pids, timeout=20)


def _wait_for_node_registered(webui_url, node_id, timeout=30):
"""Wait until `node_id` appears in the dashboard's node summary."""

def _registered():
response = requests.get(webui_url + "/nodes?view=summary")
if response.status_code != 200:
return False
summary = response.json()["data"]["summary"]
return any(node["raylet"]["nodeId"] == node_id for node in summary)

wait_for_condition(_registered, timeout=timeout)


def test_node_api_status_codes(disable_aiohttp_cache, ray_start_with_dashboard):
"""The node APIs return 4xx for client errors instead of 200/500.

`ray_start_with_dashboard` spins up a fresh Ray cluster, so this is a
single test that queries one cluster multiple times rather than a
parametrized test (which would launch a cluster per case).
"""
assert wait_until_server_available(ray_start_with_dashboard["webui_url"]) is True
webui_url = format_web_url(ray_start_with_dashboard["webui_url"])
node_id = ray_start_with_dashboard["node_id"]

# `wait_until_server_available` only checks that the HTTP server accepts
# connections. `/nodes/{node_id}` now 404s until the node shows up in the
# dashboard's node table, so wait for that before asserting on a status code.
_wait_for_node_registered(webui_url, node_id)

unknown_node_id = "8" * len(node_id)
for path, expected_status_code in [
# An unknown node ID is a client error, not a server error.
(f"/nodes/{unknown_node_id}", 404),
# An unsupported `view` is a client error, not a server error.
("/nodes?view=unknown_view", 400),
("/nodes", 400),
# Sanity check that the success paths still return 200.
(f"/nodes/{node_id}", 200),
("/nodes?view=summary", 200),
("/nodes?view=hostnamelist", 200),
]:
response = requests.get(webui_url + path)
assert response.status_code == expected_status_code, (path, response.text)
assert response.json()["result"] is (expected_status_code == 200), path


if __name__ == "__main__":
sys.exit(pytest.main(["-v", __file__]))
Loading