fix: clarify todo-note route misses
This commit is contained in:
@@ -591,6 +591,162 @@ fn content_type_is_json(content_type: &str) -> bool {
|
||||
content_type.to_ascii_lowercase().contains("json")
|
||||
}
|
||||
|
||||
const TODO_NOTE_ACTION_TYPES: &[&str] = &[
|
||||
"addTodo",
|
||||
"updateTodoTitle",
|
||||
"toggleTodoCompleted",
|
||||
"toggleTodoExpanded",
|
||||
"setAllTodosExpanded",
|
||||
"moveTodo",
|
||||
"deleteTodo",
|
||||
"renameInstance",
|
||||
"setTodoReminder",
|
||||
];
|
||||
|
||||
const TODO_NOTE_WRITABLE_ENDPOINTS: &[(&str, &str, &str)] = &[
|
||||
(
|
||||
"POST",
|
||||
"/api/instances",
|
||||
"Create a new todo list; body {name}.",
|
||||
),
|
||||
(
|
||||
"DELETE",
|
||||
"/api/instances/:instanceId",
|
||||
"Delete a todo list.",
|
||||
),
|
||||
(
|
||||
"POST",
|
||||
"/api/instances/:instanceId/actions",
|
||||
"Apply a typed action; body {action: {type, ...}}. Use the action queue pattern, not REST collection paths like /api/instances/:id/todos.",
|
||||
),
|
||||
(
|
||||
"POST",
|
||||
"/api/instances/:instanceId/undo",
|
||||
"Undo the last applied action.",
|
||||
),
|
||||
(
|
||||
"POST",
|
||||
"/api/instances/:instanceId/redo",
|
||||
"Redo the last undone action.",
|
||||
),
|
||||
];
|
||||
|
||||
fn is_todo_note_misleading_route_not_found(
|
||||
status: u16,
|
||||
content_type: &str,
|
||||
body_text: &str,
|
||||
) -> Option<Value> {
|
||||
if status != 404 || !content_type_is_json(content_type) {
|
||||
return None;
|
||||
}
|
||||
let body = serde_json::from_str::<Value>(body_text).ok()?;
|
||||
let record = body.as_object()?;
|
||||
if record.get("ok").and_then(Value::as_bool) == Some(false)
|
||||
&& record.get("error").and_then(Value::as_str)
|
||||
== Some("Todo Note is running in backend-only mode")
|
||||
{
|
||||
Some(body)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn todo_note_route_not_found_body(
|
||||
method: &Method,
|
||||
target_path: &str,
|
||||
upstream_body: Value,
|
||||
) -> Value {
|
||||
json!({
|
||||
"ok": false,
|
||||
"error": "Todo Note route not found",
|
||||
"backendOnly": true,
|
||||
"method": method.as_str(),
|
||||
"path": target_path,
|
||||
"bodyRewritten": true,
|
||||
"rewriteReason": "Todo Note catch-all 404 body was misleading; UniDesk proxy wrapped it with a structured route diagnostic. See docs/reference/cli.md and pikasTech/unidesk issue #198 for context.",
|
||||
"writableApiEndpoints": TODO_NOTE_WRITABLE_ENDPOINTS.iter().map(|(method, path, hint)| json!({ "method": method, "path": path, "hint": hint })).collect::<Vec<Value>>(),
|
||||
"actionTypes": TODO_NOTE_ACTION_TYPES,
|
||||
"hint": "Write operations use the action queue pattern: POST /api/instances/:instanceId/actions with body {action: {type, ...}}. The requested path/method is not a registered Todo Note API route; TODO_NOTE_BACKEND_ONLY only disables the upstream Vite SPA and does not lock registered API writes.",
|
||||
"issueReference": "pikasTech/unidesk#198",
|
||||
"upstreamBody": upstream_body,
|
||||
})
|
||||
}
|
||||
|
||||
async fn rewrite_todo_note_misleading_route_not_found_response(
|
||||
service: &MicroserviceConfig,
|
||||
method: &Method,
|
||||
target_path: &str,
|
||||
response: Response,
|
||||
) -> Response {
|
||||
if service.id != "todo-note" {
|
||||
return response;
|
||||
}
|
||||
let status = response.status().as_u16();
|
||||
let headers = response.headers().clone();
|
||||
let content_type = headers
|
||||
.get(header::CONTENT_TYPE)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.unwrap_or("application/octet-stream")
|
||||
.to_string();
|
||||
if status != 404 || !content_type_is_json(&content_type) {
|
||||
return response;
|
||||
}
|
||||
let bytes = to_bytes(response.into_body(), 16 * 1024 * 1024)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
let body_text = String::from_utf8_lossy(&bytes).to_string();
|
||||
let Some(upstream_body) =
|
||||
is_todo_note_misleading_route_not_found(status, &content_type, &body_text)
|
||||
else {
|
||||
let mut rebuilt = response_with_body(status, &content_type, body_text);
|
||||
for name in FORWARD_RESPONSE_HEADERS {
|
||||
if let Some(value) = headers.get(*name).and_then(|value| value.to_str().ok()) {
|
||||
add_header(&mut rebuilt, name, value);
|
||||
}
|
||||
}
|
||||
if let Some(value) = headers
|
||||
.get("x-unidesk-proxy-mode")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
{
|
||||
add_header(&mut rebuilt, "x-unidesk-proxy-mode", value);
|
||||
}
|
||||
if let Some(value) = headers
|
||||
.get("x-unidesk-upstream-proxy-mode")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
{
|
||||
add_header(&mut rebuilt, "x-unidesk-upstream-proxy-mode", value);
|
||||
}
|
||||
return rebuilt;
|
||||
};
|
||||
let mut rewritten = json_response(
|
||||
todo_note_route_not_found_body(method, target_path, upstream_body),
|
||||
404,
|
||||
);
|
||||
for name in FORWARD_RESPONSE_HEADERS {
|
||||
if let Some(value) = headers.get(*name).and_then(|value| value.to_str().ok()) {
|
||||
add_header(&mut rewritten, name, value);
|
||||
}
|
||||
}
|
||||
if let Some(value) = headers
|
||||
.get("x-unidesk-proxy-mode")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
{
|
||||
add_header(&mut rewritten, "x-unidesk-proxy-mode", value);
|
||||
}
|
||||
if let Some(value) = headers
|
||||
.get("x-unidesk-upstream-proxy-mode")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
{
|
||||
add_header(&mut rewritten, "x-unidesk-upstream-proxy-mode", value);
|
||||
}
|
||||
add_header(
|
||||
&mut rewritten,
|
||||
"x-unidesk-todo-note-route-diagnostic",
|
||||
"true",
|
||||
);
|
||||
rewritten
|
||||
}
|
||||
|
||||
fn apply_json_array_limits(body_text: String, content_type: &str, limits: &Value) -> String {
|
||||
let Some(limits) = limits.as_object() else {
|
||||
return body_text;
|
||||
@@ -2391,6 +2547,13 @@ pub async fn microservice_route(
|
||||
body_text,
|
||||
)
|
||||
.await;
|
||||
let response = rewrite_todo_note_misleading_route_not_found_response(
|
||||
&service,
|
||||
&method,
|
||||
&target_path,
|
||||
response,
|
||||
)
|
||||
.await;
|
||||
if (method == Method::GET || method == Method::HEAD)
|
||||
&& is_microservice_transient_failure_response(&response)
|
||||
{
|
||||
@@ -2417,6 +2580,79 @@ pub async fn microservice_route(
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn todo_note_route_diagnostic_matches_only_misleading_404_body() {
|
||||
let upstream = r#"{"ok":false,"error":"Todo Note is running in backend-only mode"}"#;
|
||||
let matched = is_todo_note_misleading_route_not_found(
|
||||
404,
|
||||
"application/json; charset=utf-8",
|
||||
upstream,
|
||||
)
|
||||
.expect("misleading body should match");
|
||||
assert_eq!(
|
||||
matched.get("error").and_then(Value::as_str),
|
||||
Some("Todo Note is running in backend-only mode")
|
||||
);
|
||||
assert!(
|
||||
is_todo_note_misleading_route_not_found(
|
||||
404,
|
||||
"application/json",
|
||||
r#"{"ok":false,"error":"Cannot POST /api/todos"}"#,
|
||||
)
|
||||
.is_none()
|
||||
);
|
||||
assert!(
|
||||
is_todo_note_misleading_route_not_found(200, "application/json", upstream).is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn todo_note_route_diagnostic_describes_missing_post_path() {
|
||||
let body = todo_note_route_not_found_body(
|
||||
&Method::POST,
|
||||
"/api/instances/instance_probe_bad/todos",
|
||||
json!({ "ok": false, "error": "Todo Note is running in backend-only mode" }),
|
||||
);
|
||||
assert_eq!(
|
||||
body.get("error").and_then(Value::as_str),
|
||||
Some("Todo Note route not found")
|
||||
);
|
||||
assert_eq!(body.get("backendOnly").and_then(Value::as_bool), Some(true));
|
||||
assert_eq!(body.get("method").and_then(Value::as_str), Some("POST"));
|
||||
assert_eq!(
|
||||
body.get("path").and_then(Value::as_str),
|
||||
Some("/api/instances/instance_probe_bad/todos")
|
||||
);
|
||||
assert_eq!(
|
||||
body.get("bodyRewritten").and_then(Value::as_bool),
|
||||
Some(true)
|
||||
);
|
||||
assert!(
|
||||
body.get("writableApiEndpoints")
|
||||
.and_then(Value::as_array)
|
||||
.is_some_and(|endpoints| endpoints.iter().any(|endpoint| endpoint
|
||||
.get("path")
|
||||
.and_then(Value::as_str)
|
||||
== Some("/api/instances/:instanceId/actions")))
|
||||
);
|
||||
assert!(
|
||||
body.get("actionTypes")
|
||||
.and_then(Value::as_array)
|
||||
.is_some_and(|actions| actions
|
||||
.iter()
|
||||
.any(|action| action.as_str() == Some("addTodo")))
|
||||
);
|
||||
assert_eq!(
|
||||
body.pointer("/upstreamBody/error").and_then(Value::as_str),
|
||||
Some("Todo Note is running in backend-only mode")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn urlencoding_like(value: &str) -> String {
|
||||
percent_encoding::utf8_percent_encode(value, percent_encoding::NON_ALPHANUMERIC).to_string()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user