fix: Resolve MIR phi value bug and HTTP error handling

- Fix MIR builder to correctly return phi values in if-else statements
  - Add extract_assigned_var helper to handle Program blocks
  - Update variable mapping after phi node creation
  - Fixes 'Value %14 not set' error in e2e_vm_http_client_error_result

- Fix HTTP plugin error handling for connection failures
  - Return error string instead of handle on tcp_ok=false
  - Enable proper ResultBox(Err) creation for failed connections
  - Now r.isOk() correctly returns false for connection errors

- Add VM fallback for toString() on any Box type
  - Ensures error messages can be displayed even without specific implementation

- All e2e_vm_http_client_error_result tests now pass

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Moe Charm
2025-08-23 06:51:49 +09:00
parent 3b03d001ba
commit 494a864ed2
5 changed files with 123 additions and 8 deletions

View File

@ -555,7 +555,13 @@ unsafe fn client_invoke(m: u32, id: u32, args: *const u8, args_len: usize, res:
netlog!("client.get: url={} resp_id={} tcp_ok=false", url, resp_id); netlog!("client.get: url={} resp_id={} tcp_ok=false", url, resp_id);
} }
// No stub enqueue in TCP-only design // No stub enqueue in TCP-only design
write_tlv_handle(T_RESPONSE, resp_id, res, res_len) if tcp_ok {
write_tlv_handle(T_RESPONSE, resp_id, res, res_len)
} else {
// Encode error string; loader interprets returns_result=true methods' string payload as Err
let msg = format!("connect failed for {}:{}{}", host, port, if path.is_empty() { "" } else { &path });
write_tlv_string(&msg, res, res_len)
}
} }
M_CLIENT_POST => { M_CLIENT_POST => {
// args: TLV String(url), Bytes body // args: TLV String(url), Bytes body
@ -598,7 +604,12 @@ unsafe fn client_invoke(m: u32, id: u32, args: *const u8, args_len: usize, res:
netlog!("client.post: url={} resp_id={} tcp_ok=false body_len={}", url, resp_id, body.len()); netlog!("client.post: url={} resp_id={} tcp_ok=false body_len={}", url, resp_id, body.len());
} }
// No stub enqueue in TCP-only design // No stub enqueue in TCP-only design
write_tlv_handle(T_RESPONSE, resp_id, res, res_len) if tcp_ok {
write_tlv_handle(T_RESPONSE, resp_id, res, res_len)
} else {
let msg = format!("connect failed for {}:{}{} (body_len={})", host, port, if path.is_empty() { "" } else { &path }, body_len);
write_tlv_string(&msg, res, res_len)
}
} }
_ => E_INV_METHOD, _ => E_INV_METHOD,
} }

View File

@ -1116,6 +1116,11 @@ impl VM {
} }
} }
// Generic fallback: toString for any Box type
if method == "toString" {
return Ok(Box::new(StringBox::new(box_value.to_string_box().value)));
}
// StringBox methods // StringBox methods
if let Some(string_box) = box_value.as_any().downcast_ref::<StringBox>() { if let Some(string_box) = box_value.as_any().downcast_ref::<StringBox>() {
match method { match method {

View File

@ -523,6 +523,8 @@ impl MirBuilder {
// Build then branch // Build then branch
self.current_block = Some(then_block); self.current_block = Some(then_block);
self.ensure_block_exists(then_block)?; self.ensure_block_exists(then_block)?;
// Keep a copy of AST for analysis (phi for variable reassignment)
let then_ast_for_analysis = then_branch.clone();
let then_value = self.build_expression(then_branch)?; let then_value = self.build_expression(then_branch)?;
if !self.is_current_block_terminated() { if !self.is_current_block_terminated() {
self.emit_instruction(MirInstruction::Jump { target: merge_block })?; self.emit_instruction(MirInstruction::Jump { target: merge_block })?;
@ -531,8 +533,9 @@ impl MirBuilder {
// Build else branch // Build else branch
self.current_block = Some(else_block); self.current_block = Some(else_block);
self.ensure_block_exists(else_block)?; self.ensure_block_exists(else_block)?;
let else_value = if let Some(else_ast) = else_branch { let (else_value, else_ast_for_analysis) = if let Some(else_ast) = else_branch {
self.build_expression(else_ast)? let val = self.build_expression(else_ast.clone())?;
(val, Some(else_ast))
} else { } else {
// No else branch, use void // No else branch, use void
let void_val = self.value_gen.next(); let void_val = self.value_gen.next();
@ -540,7 +543,7 @@ impl MirBuilder {
dst: void_val, dst: void_val,
value: ConstValue::Void, value: ConstValue::Void,
})?; })?;
void_val (void_val, None)
}; };
if !self.is_current_block_terminated() { if !self.is_current_block_terminated() {
self.emit_instruction(MirInstruction::Jump { target: merge_block })?; self.emit_instruction(MirInstruction::Jump { target: merge_block })?;
@ -558,9 +561,33 @@ impl MirBuilder {
(else_block, else_value), (else_block, else_value),
], ],
})?; })?;
// Heuristic: If both branches assign the same variable name, bind that variable to the phi result
let assigned_var_then = Self::extract_assigned_var(&then_ast_for_analysis);
let assigned_var_else = else_ast_for_analysis.as_ref().and_then(|a| Self::extract_assigned_var(a));
if let (Some(a), Some(b)) = (assigned_var_then, assigned_var_else) {
if a == b {
self.variable_map.insert(a, result_val);
}
}
Ok(result_val) Ok(result_val)
} }
/// Extract assigned variable name from an AST node if it represents an assignment to a variable.
/// Handles direct Assignment and Program with trailing single-statement Assignment.
fn extract_assigned_var(ast: &ASTNode) -> Option<String> {
match ast {
ASTNode::Assignment { target, .. } => {
if let ASTNode::Variable { name, .. } = target.as_ref() { Some(name.clone()) } else { None }
}
ASTNode::Program { statements, .. } => {
// Inspect the last statement as the resulting value of the block
statements.last().and_then(|st| Self::extract_assigned_var(st))
}
_ => None,
}
}
/// Emit an instruction to the current basic block /// Emit an instruction to the current basic block
pub(super) fn emit_instruction(&mut self, instruction: MirInstruction) -> Result<(), String> { pub(super) fn emit_instruction(&mut self, instruction: MirInstruction) -> Result<(), String> {

View File

@ -590,9 +590,14 @@ impl PluginBoxV2 {
} }
6 | 7 => { // String/Bytes 6 | 7 => { // String/Bytes
let s = String::from_utf8_lossy(payload).to_string(); let s = String::from_utf8_lossy(payload).to_string();
let val: Box<dyn NyashBox> = Box::new(StringBox::new(s));
if dbg_on() { eprintln!("[Plugin→VM] return str/bytes len={} (returns_result={})", size, returns_result); } if dbg_on() { eprintln!("[Plugin→VM] return str/bytes len={} (returns_result={})", size, returns_result); }
if returns_result { Some(Box::new(crate::boxes::result::NyashResultBox::new_ok(val)) as Box<dyn NyashBox>) } else { Some(val) } if returns_result {
// Heuristic: for Result-returning methods, string payload represents an error message
let err = crate::exception_box::ErrorBox::new(&s);
Some(Box::new(crate::boxes::result::NyashResultBox::new_err(Box::new(err))) as Box<dyn NyashBox>)
} else {
Some(Box::new(StringBox::new(s)) as Box<dyn NyashBox>)
}
} }
9 => { 9 => {
if dbg_on() { eprintln!("[Plugin→VM] return void (returns_result={})", returns_result); } if dbg_on() { eprintln!("[Plugin→VM] return void (returns_result={})", returns_result); }

View File

@ -4,6 +4,8 @@ use nyash_rust::parser::NyashParser;
use nyash_rust::runtime::plugin_loader_v2::{init_global_loader_v2, get_global_loader_v2}; use nyash_rust::runtime::plugin_loader_v2::{init_global_loader_v2, get_global_loader_v2};
use nyash_rust::runtime::box_registry::get_global_registry; use nyash_rust::runtime::box_registry::get_global_registry;
use nyash_rust::runtime::PluginConfig; use nyash_rust::runtime::PluginConfig;
use nyash_rust::runtime::NyashRuntime;
use nyash_rust::backend::VM;
fn try_init_plugins() -> bool { fn try_init_plugins() -> bool {
if !std::path::Path::new("nyash.toml").exists() { return false; } if !std::path::Path::new("nyash.toml").exists() { return false; }
@ -100,3 +102,68 @@ hv + ":" + body
assert!(s.starts_with("A:")); assert!(s.starts_with("A:"));
assert!(s.contains("OK-LONG")); assert!(s.contains("OK-LONG"));
} }
#[test]
fn e2e_vm_http_client_error_result() {
std::env::set_var("NYASH_NET_LOG", "1");
std::env::set_var("NYASH_NET_LOG_FILE", "net_plugin.log");
if !try_init_plugins() { return; }
// No server on 8099 → should produce Err result
let code = r#"
local cli, r, ok, result
cli = new HttpClientBox()
r = cli.get("http://127.0.0.1:8099/nope")
ok = r.isOk()
if ok {
result = "unexpected_ok"
} else {
result = r.getError().toString()
}
result
"#;
let ast = NyashParser::parse_from_string(code).expect("parse failed");
let runtime = NyashRuntime::new();
let mut compiler = nyash_rust::mir::MirCompiler::new();
let compile_result = compiler.compile(ast).expect("mir compile failed");
let mut vm = VM::with_runtime(runtime);
let result = vm.execute_module(&compile_result.module).expect("vm exec failed");
let s = result.to_string_box().value;
assert!(s.contains("Error") || s.contains("unexpected_ok") == false);
}
#[test]
fn e2e_vm_http_empty_body() {
std::env::set_var("NYASH_NET_LOG", "1");
std::env::set_var("NYASH_NET_LOG_FILE", "net_plugin.log");
if !try_init_plugins() { return; }
let code = r#"
local srv, cli, r, resp, req, body
srv = new HttpServerBox()
srv.start(8087)
cli = new HttpClientBox()
r = cli.get("http://localhost:8087/empty")
req = srv.accept().get_value()
resp = new HttpResponseBox()
resp.setStatus(204)
// no body written
req.respond(resp)
resp = r.get_value()
body = resp.readBody()
body
"#;
let ast = NyashParser::parse_from_string(code).expect("parse failed");
let runtime = NyashRuntime::new();
let mut compiler = nyash_rust::mir::MirCompiler::new();
let compile_result = compiler.compile(ast).expect("mir compile failed");
let mut vm = VM::with_runtime(runtime);
let result = vm.execute_module(&compile_result.module).expect("vm exec failed");
assert_eq!(result.to_string_box().value, "");
}