Skip to content

fix(api): name the callee in call_function/3 errors#383

Open
davydog187 wants to merge 3 commits into
mainfrom
fix/call-function-undefined-error
Open

fix(api): name the callee in call_function/3 errors#383
davydog187 wants to merge 3 commits into
mainfrom
fix/call-function-undefined-error

Conversation

@davydog187

Copy link
Copy Markdown
Contributor

Problem

Reported in #92. The rich in-Lua error path is already excellent (it names globals/locals/upvalues/fields/methods, renders source context, stack traces, and suggestions). The gap was entirely in the programmatic Lua.call_function/3 boundary, which resolved a name itself and hand-rolled an error string that inspected the resolved value instead of the name that was looked up:

{:error, "undefined function '#{inspect(func)}'", lua.state}  # reports nil, not the name

So Lua.call_function(lua, [:bar], []) returned undefined function 'nil'.

Fix

Both fallbacks in lib/lua.ex now build the same TypeError the VM's :call opcode raises, via a shared Executor.call_type_error/3. Returned as a struct (not raised), so it flows through both existing contracts unchanged:

Call Before After
call_function(lua, [:bar], []) {:error, "undefined function 'nil'", …} {:error, "attempt to call a nil value (global 'bar')", …}
call_function(lua, [:x], []) (x=5) {:error, "undefined function '5'", …} {:error, "attempt to call a number value (global 'x')", …}
call_function!(lua, [:bar], []) Lua runtime error: undefined function 'nil' Full render: message + Suggestion: block
nested [:t, :missing] undefined function 'nil' attempt to call a nil value (field 'missing')

The terse (pcall-parity) and rich (call_function!) variants now match the in-Lua path exactly.

Tests

  • Resurrected loading a file that calls an undefined function as a real load_file! integration test (the undefined_function.lua fixture already existed).
  • Covered the programmatic undefined/non-function path with new tests in call_function_error_value_test.exs.
  • Deleted 6 redundant/invalid Luerl-era commented-out stubs whose paths error_messages_test.exs already covers, or whose premises are wrong for Lua 5.3.

All 2579 tests pass; mix format clean.

Closes #92

Programmatic Lua.call_function/3 resolved a name to a value itself and
hand-rolled its own error string, inspecting the *resolved* value rather
than the name that was looked up. Calling an undefined function reported
"undefined function 'nil'" — naming the thing the caller doesn't care
about and dropping the one they asked for.

Route both fallbacks through the same TypeError the VM's :call opcode
raises, via a shared Executor.call_type_error/3. Returned as a struct
(not raised) so it flows through both existing contracts: call_function/3
hands back the terse pcall-parity value ("attempt to call a nil value
(global 'bar')") while call_function!/3 re-raises it for the rich render
with a Suggestion block.

Resurrect the load_file! undefined-function integration test, cover the
programmatic non-function path in call_function_error_value_test.exs, and
delete the redundant/invalid Luerl-era stubs whose paths error_messages_test
already covers or whose premises are wrong for Lua 5.3.

Closes #92
The programmatic call_function/3 boundary reports a __call-bearing
table as "attempt to call a table value" rather than invoking the
metamethod, unlike the in-Lua :call opcode. Document this as an
intentional scope boundary so the "matches in-Lua exactly" framing
isn't read literally.
Comment on lines +79 to +83
assert {:error, reason, %Lua{}} = Lua.call_function(lua, [:bar], [])

# Regression: previously reported the resolved value ("undefined
# function 'nil'"). It must name the requested global instead.
assert reason == "attempt to call a nil value (global 'bar')"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be returning the exception struct uniformly instead of returning a string. it should be up to the user to call Exception.message(). We should fix this across all of the public facing APIs so it consistently returns the excception struct

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d6164a5. Both public {:error, _} APIs now hand back the exception struct uniformly:

  • call_function/3{:error, exception, lua} where exception is the VM struct (Lua.VM.RuntimeError/TypeError/ArgumentError). Per our discussion I wrapped everything, including user-thrown error() values — error(42) comes back as %RuntimeError{value: 42}, tables/nil/false likewise on :value — so pcall-parity data survives on the struct. In-Lua pcall/xpcall still project the raw §6.1 value via ProtectedCall.error_value/1; only the programmatic boundary changed.
  • parse_chunk/1{:error, %Lua.CompilerException{}} (was {:error, [String.t()]}); diagnostics on :errors.

Callers now own rendering via Exception.message/1. Doctests, the chunks guide, this regression suite, and a breaking-before-1.0 CHANGELOG entry are updated. Full suite (2577) + format + dialyzer green.

Those are the only two public APIs that returned error tuples — everything else (eval!, load_file!, call_function!) already raises, so callers get the struct there too.

…nk/1

The public error-tuple APIs pre-rendered their errors to a string at the
boundary, forcing one representation on callers and discarding the
structured exception. Return the exception struct uniformly instead so
callers own rendering (Exception.message/1) and can pattern-match the
concrete error.

- call_function/3 now returns {:error, exception, lua} carrying the VM
  struct (RuntimeError/TypeError/ArgumentError). The raised Lua value
  (error(42), a table, nil, false) is preserved on the struct's :value,
  matching what pcall hands back inside Lua. In-Lua pcall/xpcall still
  project the raw §6.1 value via ProtectedCall.error_value/1.
- parse_chunk/1 now returns {:error, %Lua.CompilerException{}} instead of
  {:error, [String.t()]}; :errors holds the formatted diagnostics.

Updates doctests, guides, the CHANGELOG (breaking-before-1.0), and the
error-value regression suite to the struct contract.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better error for undefined function

1 participant