getAccountType crashes on latest nightly

[TracerDS]

Describe the bug

When you download nightly version, the getAccountType will crash if you dont provide an account as an argument to it.
Cloning mta repo and building it manually will properly detect invalid arguments provided to the getAccountType function.

Steps to reproduce

1. Download mta server from nightly
2. Run the server with the runcode resource
3. Running getAccountType() - Crash
4. Running getAccountType('') - Crash
5. Running getAccountType(false) - Crash
6. Running getAccountType(validAccount) - Doesn't crash

Version

Server v1.6-release-22470

Additional context

No response

Relevant log output

No response

Security Policy

• I have read and understood the Security Policy and this issue is not security related.

[TracerDS]

still crashing on r22485

[theSarrum]

It crashes only on x64 builds (not sure about Linux).
And it seems that a similar issue occurs with fileGetContents.

[TheNormalnij]

I think CAccount isn't an element and argument parser doesn't generate checks for it

[TracerDS]

but then why would compiling directly work fine?

[TheNormalnij]

Hmm. I checked on windows x64 Release and it crashes with getAccountType('lol')
It crashes at error handling. I believe std::optional<std::string> is the clue

[TheNormalnij]

Is there any reason to use std::optional<std::string> if the function throws an error?

[TracerDS]

Is there any reason to use std::optional<std::string> if the function throws an error?

if its pcalled (or it throws warning) then it should return false, hence the std::optional

[TracerDS]

I'd guess std::optional<std::string> would crash. pathListDir doesnt crash and only 2 other functions use std::optional

It crashes at error handling. I believe std::optional<std::string> is the clue

yes. pathListDir doesnt crash and only 2 other functions use std::optional, so...
It would make sense why only these 2 functions crash, but why would it crash on release?

[TheNormalnij]

if its pcalled (or it throws warning) then it should return false, hence the std::optional

pcall returns true and functions result when success and only false in case of errors. Is this true?
getAccountType doesn't crash when it returns std::string

[TracerDS]

pcall returns true and functions result when success and only false in case of errors. Is this true? getAccountType doesn't crash when it returns std::string

when function doesnt error then pcall return true + any arguments returned from the function.
when function errors, pcall returns false + error message

[TheNormalnij]

So there is no reason to use std::optinal

[TracerDS]

in theory optional would return null in this case
Unless it errors, then it would return nothing ;c
Still, it shouldnt crash at all. Something is broken with the release/nightly version

[TheNormalnij]

We can return an empty string or "unknown".
Maybe you could add lua enum definition for account type and the code won't compile unless all account types have string representation. But im not sure

[TracerDS]

We can return an empty string or "unknown". Maybe you could add lua enum definition for account type and the code won't compile unless all account types have string representation. But im not sure

Or we could focus on fixing std::optional on release builds

[TheNormalnij]

Unless it errors, then it would return nothing ;c

There is ArgumentParserWarn for that.

[TracerDS]

It would throw a warning instead of hard error, I dont think getAccountType should throw a soft warning instead of hard error

[TheNormalnij]

I dont get why there should be std::optional

[TracerDS]

I dont get why there should be std::optional

🤔 actually, you're right. Appendum coming right up.

[TheNormalnij]

fileGetContents could be fixed with ArgumentParserWarn<nullptr, fileGetContents> i think

[TracerDS]

+ some refactoring ig