Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lua] exit code 0 in case of error #10979

Closed
sebthom opened this issue Feb 24, 2023 · 12 comments
Closed

[lua] exit code 0 in case of error #10979

sebthom opened this issue Feb 24, 2023 · 12 comments
Milestone

Comments

@sebthom
Copy link
Contributor

sebthom commented Feb 24, 2023

A failing Haxe program targeting/running on Lua will exit with error code 0 instead of a non-zero exit code. For all other targets an unhandled error/exception will result in a non-zero exit code.

Example:

class Main {
  static function main() {
    throw "foo";
  }
}
> haxe --lua test.lua -p . -main Main
> lua test.lua && echo success %errorlevel% || echo error %errorlevel%
runtime error:
 foo
stack traceback:
        test.lua:135: in function <test.lua:127>
        [C]: in function 'error'
        test.lua:545: in function <test.lua:544>
        [C]: in function 'xpcall'
        test.lua:1053: in main chunk
        [C]: in ?
success 0

Tested with Haxe 4.2.5, Lua 5.2.4 on Windows 10.

@sebthom
Copy link
Contributor Author

sebthom commented Feb 24, 2023

I suspect the issue is that the entry-point is wrapped in a xpcall which has _hx_error registered as its error handler

haxe/src/generators/genlua.ml

Lines 2147 to 2160 in 8d4f2d7

spr ctx "_G.xpcall(";
let luv_run =
(* Runs libuv loop if needed *)
mk_lua_code ctx.com.basic "_hx_luv.run()" [] ctx.com.basic.tvoid Globals.null_pos
in
let fn =
{
tf_args = [];
tf_type = com.basic.tvoid;
tf_expr = mk (TBlock [e;luv_run]) com.basic.tvoid e.epos;
}
in
gen_value ctx { e with eexpr = TFunction fn; etype = TFun ([],com.basic.tvoid) };
spr ctx ", _hx_error)";

_hx_error defined here:

function _hx_error(obj)
if obj.value then
_G.print("runtime error:\n " .. _hx_tostring(obj.value));
else
_G.print("runtime error:\n " .. tostring(obj));
end
if _G.debug and _G.debug.traceback then
_G.print(debug.traceback());
end
however only prints the error instead of rethrowing it or exiting the process with an error code.

@sebthom
Copy link
Contributor Author

sebthom commented Feb 28, 2023

@jdonaldson @inklit @tobil4sk @RealyUniqueName since worked on the Lua target I'd like to ask for your opinion on what would the best way to solve this. I can provide a PR but I am unsure about the right approach. Any help is highly appreciated.

@tobil4sk
Copy link
Member

tobil4sk commented Mar 1, 2023

I've done very minimal work with the lua target, but I think it would be fine just to add a _G.os.exit(1) call at the end of the _hx_error function. _hx_error is not used anywhere else so it shouldn't break anything. There is not much else to compare to since most other targets don't do any error handling like this.

@sebthom
Copy link
Contributor Author

sebthom commented Mar 1, 2023

I could imagine the reason why this was added is that lua scripts often don't run stand alone but are plugins inside other programs. so if we add os. exit() and a lua script is used in an embedded lua interpreter and then not only that lua plugin fails but the whole app may crash/exit. So I wonder if we couldn't just remove the whole error handling so the error can bubble up and the callee can decide what to do.

@tobil4sk
Copy link
Member

tobil4sk commented Mar 7, 2023

I guess one way to solve this is to have a compiler flag that controls whether this extra error handling is added. This way, it works both for embeddable scripts and for standalone scripts.

Really, all it does it it makes uncaught exceptions more readable in the terminal:

runtime error:
 error
stack traceback:
        bin/lua/main.lua:136: in function '_hx_error'
        [C]: in function 'error'
        bin/lua/main.lua:533: in field 'main'
        bin/lua/main.lua:1070: in function <bin/lua/main.lua:1069>
        [C]: in function 'xpcall'
        bin/lua/main.lua:1069: in main chunk
        [C]: in ?

vs

lua: { __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:<...>, value:<...>, __skipStack:<...>, __nativeStack:<...>, __nativeException:<...> } } } } } }

@sebthom
Copy link
Contributor Author

sebthom commented Mar 7, 2023

Compiler flag sounds like a reasonable idea. Any suggestions? Something like lua-exit-on-error?

@kLabz
Copy link
Contributor

kLabz commented Mar 7, 2023

lua.standalone ?

@sebthom
Copy link
Contributor Author

sebthom commented Mar 7, 2023

Alright, I created a PR. Can you please have a look.

@Simn Simn added this to the Later milestone Mar 24, 2023
@sebthom
Copy link
Contributor Author

sebthom commented Mar 24, 2023

@Simn since we already have more or less ready PR #11008 could this be included in the 4.3 release?

@Simn
Copy link
Member

Simn commented Mar 24, 2023

Yeah, I'll go through PRs tomorrow, just did issues today and I missed some with linked PRs.

@tobil4sk
Copy link
Member

I think we can close this, now that #11008 is merged.

@tobil4sk
Copy link
Member

tobil4sk commented Apr 3, 2023

I decided to have another look at this, because I got curious why without the error handler function haxe programs print out this unreadable mess:

lua: { __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:error, value:error, __skipStack:2, __nativeStack:[...], __nativeException:{ __exceptionMessage:<...>, value:<...>, __skipStack:<...>, __nativeStack:<...>, __nativeException:<...> } } } } } }

So I read up on how lua handles error values: http://www.lua.org/manual/5.4/manual.html#7
It should be checking for the __tostring() meta method, which gets mapped to toString() in haxe generated types. So in theory this should print lua: test instead of the mess above, but since toString gets dce'd, it prints the entire table instead. This can be fixed by marking toString with @:keep for lua.

@sebthom I then had another look at your original suggestion of rethrowing the error, so I modified the error handler to return a new error object with __tostring() which just returns the exception message along with the traceback() value. Then I changed the main code to rethrow the error if the main xpcall returns an error. So now, the original error message and call stack are preserved for both uncaught haxe exceptions and uncaught native exceptions.

This method solves the original problem of returning the correct error code, without compromising on error message readability and without using _G.os.exit, so it is safer and should work for both embedded scripts and for standalone scripts, making the flag redundant. Let me know what you think: #11082

(Also, for reference the reason the stack trace is missing from custom errors by default is because of: http://lua-users.org/lists/lua-l/2014-10/msg00115.html. This is why we need the custom error handler.)

tobil4sk added a commit to tobil4sk/haxec that referenced this issue Apr 3, 2023
kLabz pushed a commit that referenced this issue Apr 4, 2023
* [lua] Clean up uncaught error handling

Removes usage of os.exit, which makes it safer and applicable both
to lua scripts embedded in another application and standalone scripts.
This removes the need for the lua-standalone flag.

* [lua] Mark Exception.toString with @:keep

If it is dce'd, we get a mess when lua prints an uncaught error

* [lua] Prevent error handler polluting error stack

* Add tests for #10979

* Prevent test failures on lua 5.1

Lua 5.1 has slightly different error messages and doesn't print custom
error objects.

* Minor fix
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

No branches or pull requests

4 participants