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

BizHawkClient: Add some handling for non-string errors #2656

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

Zunawe
Copy link
Collaborator

@Zunawe Zunawe commented Jan 2, 2024

What is this fixing or adding?

Someone reported an error

ERROR: .\json.lua:115: unexpected type 'function'
Consider reporting this crash.

NLua.Exceptions.LuaScriptException: tableConnection to client closed
Client timed out

Essentially the pcall for process_request caught an error, and that error was type function, so when the script tried to convert it to JSON to tell the client there was an error, it caused another error.

The actual error was an out of bounds write on GBA, and I guess BizHawk's response is to just throw your own function back at you when you cause an error. So in the case of

local status, response = pcall(process_request, req)

if process_request does an out of bounds write on GBA, response will be set to process_request. I tried letting it bubble up in BizHawk to see if maybe they used it to print meaningful information about where you did something wrong, but you ultimately just get NLua.Exceptions.LuaException: unprotected error in call to Lua API (0). So I don't really think there's anything more to be gleaned from the fact that the error is a function.

Out of bounds reads on GBA behave similarly.

How was this tested?

Triggering the error intentionally with Pokemon Emerald and seeing it get passed back to the client as expected.

Didn't do any testing on other systems.

Copy link
Member

@Berserker66 Berserker66 left a comment

Choose a reason for hiding this comment

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

I wonder if there's any way to get something meaningful, other than "unknown error".

@Berserker66 Berserker66 merged commit bf17582 into ArchipelagoMW:main Jan 2, 2024
7 checks passed
@Zunawe Zunawe deleted the bhc-err branch February 17, 2024 22:18
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
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.

2 participants