Page MenuHomeWildfire Games

Report GUI object script handler error stack
ClosedPublic

Authored by elexis on Feb 8 2020, 12:14 AM.

Details

Summary

If a script handler assigned to a GUI object calls another script handler assigned to a GUI object, and that latter handler triggers a script error,
then only the error of the first script handler will be reported, but not the one that actually triggered that error.

This patch changes the reporting behavior to report the error where it first occurred, thus allowing the developer to see the line of code that fails.

Test Plan

For example when opening a GUI page (onPress script handler) that somewhere assigns a dropdown selection (onSelectionChange) and the selection change handler triggers an error, it will only complain about the user clicking the page-open button.

Index: binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/GameSettingControlDropdown.js
===================================================================
--- binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/GameSettingControlDropdown.js	(revision 23480)
+++ binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/GameSettingControlDropdown.js	(working copy)
@@ -48,6 +48,7 @@
 
 	onSelectionChangeSuper()
 	{
+		throw new Error();
 		if (!this.isInGuiUpdate)
 			this.onSelectionChange(this.dropdown.selected);
 	}

Without this patch:

ERROR: JavaScript error: gui/gamesetup/Pages/GameSetupPage/GameSettings/GameSettingControlDropdown.js line 42
Error: Errors executing script event "SelectionChange"

You see that this reporting behavior is problematic, because the developer can only see that a function assigned to onSelectionChange somewhere errored out, but not where it did.

With this patch:

ERROR: JavaScript error: gui/gamesetup/Pages/GameSetupPage/GameSettings/GameSettingControlDropdown.js line 51

With the patch one gets an exact line where the error happens (this is the inserted line).

Notice that this behavior is the same as in other calls with all other JS_CallFunction calls.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Feb 8 2020, 12:14 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/gui/ObjectBases/IGUIObject.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1728/display/redirect

bb added a subscriber: bb.May 31 2020, 10:57 PM

Needs a rebase, appears to be equivalent to rP8997

wraitii added a subscriber: wraitii.Jun 1 2020, 6:55 AM

I independently rediscovered this on D2768. ReportError seems rather useless (particularly since JSNative functions can return false to indicate failure), and it is removed in later SM versions.

I think I have found a few other cases in D2768 which should also be changed.

wraitii accepted this revision.Jun 1 2020, 6:55 AM
This revision is now accepted and ready to land.Jun 1 2020, 6:55 AM
wraitii updated this revision to Diff 12300.Sun, Jun 14, 11:18 AM

Rebased and slight upstreaming from D2768

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2428/display/redirect

This revision was landed with ongoing or failed builds.Sun, Jun 14, 12:35 PM
This revision was automatically updated to reflect the committed changes.