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

Better Plugin Errors #830

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Better Plugin Errors #830

wants to merge 1 commit into from

Conversation

bjornbytes
Copy link
Owner

Not sure if this is a good idea, but currently when you try to require plugins and they aren't packaged right, they are pretty much silently skipped, and you'll just get a regular "module not found" error without any extra info.

Specifically, package.loadlib returns nil plus an error message and we don't do anything with that error message. We could return that error message from the lovr.filesystem searcher, which would look like this on Android:

main.lua:1: module 'mylib' not found:
	no field package.preload['mylib']
	failed to load plugin: dlopen failed: cannot locate symbol "lua_pushinteger" referenced by "/data/app/~~bFAnAwWdtkqkrPaReQBFsg==/org.lovr.app-luJnKjeMqIXUMbxDRw8vPw==/base.apk!/lib/arm64-v8a/mylib.so"...
	failed to load all-in-one plugin: dlopen failed: cannot locate symbol "lua_pushinteger" referenced by "/data/app/~~bFAnAwWdtkqkrPaReQBFsg==/org.lovr.app-luJnKjeMqIXUMbxDRw8vPw==/base.apk!/lib/arm64-v8a/mylib.so"...
	no file './mylib.lua'
	no file '/usr/local/share/luajit-2.1.0-beta3/mylib.lua'
	no file '/usr/local/share/lua/5.1/mylib.lua'
	no file '/usr/local/share/lua/5.1/mylib/init.lua'
	no file './mylib.so'
	no file '/usr/local/lib/lua/5.1/mylib.so'
	no file '/usr/local/lib/lua/5.1/loadall.so'

Here we can see that the plugin failed to load because lua_pushinteger is missing, maybe the plugin failed to link against the Lua library when it was compiled.

It basically takes the dlerror error message returned by package.loadlib and includes it like all of the other failure messages. They are kind of noisy though, maybe it should just say "failed to load plugin from luaopen_mylib in mylib.so" and log the full error instead?

Note: There are already some ways to get extended library errors today. On Linux you can use LD_DEBUG and on Android you can do db shell setprop debug.ld.all dlerror,dlopen. It's easy to forget that these options exist though, and they require hunting through logs.

@bjornbytes
Copy link
Owner Author

Yeah, the error message is way too long, it breaks the error screen.

package.loadlib returns a 3rd value indicating whether it failed to load the library ("open") or failed to load the symbol ("init"). It also can be "absent" if Lua is compiled without dlopen support.

I do think it would be good to return some reason in the list on failure, like:

  • For "open" errors, "no plugin 'a/b/c.so'" or "no plugin 'a.so'" for all-in-one.
  • For "init" erorrs, "no symbol 'luaopen_c' in plugin 'a/b/c.so" or "no symbol 'luaopen_b_c' in plugin 'a.so'" for all-in-one.

This at least might help you debug issues with dots and hyphens in the plugin name, and tell you whether or not the library is getting opened successfully.

I don't really want to log the full error message, because these will get spammed whenever you fail to load any module. I guess we can just document these LD_DEBUG/debug.ld methods on the Plugins page.

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.

1 participant