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

Correctly print cmdline errors with braces #3880

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

HebaruSan
Copy link
Member

Problem

While I was testing some edge case install stanzas for @JonnyOThan, this metadata:

    "install": {
        "file": "Astrogator",
        "install_to": "GameRoot/KSP_x64_Data"
    },

gave me this error:

$ ckan.exe install -c Astrogator-v1.0.0.ckan

Unhandled Exception: System.FormatException: Input string was not in a correct format.
   at System.Text.StringBuilder.FormatError()
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.Format(IFormatProvider provider, String format, Object[] args)
   at System.IO.TextWriter.WriteLine(String format, Object[] arg)
   at System.IO.TextWriter.SyncTextWriter.WriteLine(String format, Object[] arg)
   at CKAN.CmdLine.ConsoleUser.RaiseError(String message, Object[] args)
   at CKAN.CmdLine.Install.RunCommand(GameInstance instance, Object raw_options)
   at CKAN.CmdLine.MainClass.RunSimpleAction(Options cmdline, CommonOptions options, String[] args, IUser user, GameInstanceManager manager)
   at CKAN.CmdLine.MainClass.Execute(GameInstanceManager manager, CommonOptions opts, String[] args)
   at CKAN.CmdLine.MainClass.Main(String[] args)

Cause

The root cause is that my .ckan file was formatted wrong; the install property has to be an array, but mine was an object, so the attempt to deserialize failed.

However, the error message about that was not printed because there are places in CmdLine where a message string from an exception thrown by core code or third party libraries is passed as the first parameter of IUser.RaiseError, which always interprets that parameter as a format string (see #2111). If such a string contains { and } (as is particularly likely for JSON deserialization problems), these will be interpreted as parameter interpolation tokens and the string formatting will fail.

Changes

  • Now when we want to print an exception message, the first parameter is always {0}, to prevent the message from being interpreted as a format string. The original root cause error is printed:
    $ install -c Astrogator-v1.0.0.ckan
    JSON deserialization error: Cannot deserialize the current JSON object (e.g. {"name":"value"}) into type 'CKAN.ModuleInstallDescriptor[]' because the type requires a JSON array (e.g. [1,2,3]) to deserialize correctly.
    To fix this error either change the JSON to a JSON array (e.g. [1,2,3]) or change the deserialized type so that it is a normal .NET type (e.g. not a primitive type like integer, not a collection type like an array or List<T>) that can be deserialized from a JSON object. JsonObjectAttribute can also be added to the type to force it to deserialize from a JSON object.
    Path 'install', line 67, position 17.: Cannot deserialize the current JSON object (e.g. {"name":"value"}) into type 'CKAN.ModuleInstallDescriptor[]' because the type requires a JSON array (e.g. [1,2,3]) to deserialize correctly.
    To fix this error either change the JSON to a JSON array (e.g. [1,2,3]) or change the deserialized type so that it is a normal .NET type (e.g. not a primitive type like integer, not a collection type like an array or List<T>) that can be deserialized from a JSON object. JsonObjectAttribute can also be added to the type to force it to deserialize from a JSON object.
    Path 'install', line 67, position 17.
    Usage: ckan install [--with-suggests] [--with-all-suggests] [--no-recommends] [--headless] Mod [Mod2, ...]
    
  • An untranslated error message is internationalized.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Cmdline Issues affecting the command line labels Aug 14, 2023
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

That'll do it! Thanks @HebaruSan

@HebaruSan HebaruSan merged commit 9d0b75e into KSP-CKAN:master Aug 15, 2023
10 checks passed
@HebaruSan HebaruSan deleted the fix/cmdline-err-fmt branch August 15, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Cmdline Issues affecting the command line Easy This is easy to fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants