RPTools / maptool

Virtual Tabletop for playing roleplaying games with remote players or face to face.
http://rptools.net
GNU Affero General Public License v3.0
800 stars 262 forks source link

rest - return result vs ParserException #4890

Closed Jmr3366 closed 2 months ago

Jmr3366 commented 2 months ago

Identify the Bug or Feature request

Closes #2489 rest commands throw parser exceptions and do not provide a return result that can be checked for failure.

Description of the Change

rest commands that fail, will now provide a return of the same error previously provided through ParserException. This can provide a graceful return for macro writers to check and respond accordingly.

Possible Drawbacks

With this feature, macros that do not perform error checking on rest returns, where previously the macro would break due to the ParserException, will now continue execution. This will most likely cause the macro to fail due to bad/wrong data types (eg. errors below vs expected data).

Release Notes

Example of failures: [r: rest.get(url)] on a failure / timeout: Unable to process function "{0}", HTTP Status Code: {1} on bad response: Unable to process function "{0}", An Exception has occurred: {1}


This change is Reviewable

cwisniew commented 2 months ago

The feature request asks for the status to be returned in json format if it fails, this came will just return the translated string that would appear in the error message displayed when reporting the exception

Jmr3366 commented 2 months ago

It is not "What they'd like", you are correct. However it is adequate by allowing macro handling and does respond to the specific points mentioned; if it's a timeout or something else.

cwisniew commented 2 months ago

It is not "What they'd like", you are correct. However it is adequate by allowing macro handling and does respond to the specific points mentioned; if it's a timeout or something else.

If it is a translated string then how will coders know unless the include checks for all the different translations? I don't think I can approve this based on the feature request