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

F3D exits with error code 0 (success) despite an error having occurred (invalid json config file) #1103

Closed
mrdeveloperdude opened this issue Dec 17, 2023 · 7 comments · Fixed by #1111
Assignees
Milestone

Comments

@mrdeveloperdude
Copy link

mrdeveloperdude commented Dec 17, 2023

Describe the bug

When running f3d with an invalid json file as input to the --config commandline parameter, it exits with an error message output on stderr but returns exit code 0 to the shell, which signifies success.

The error message that f3d outputs looks like this:

"/tmp/myconfig.json" is not a file of a supported file format.
This is a headless build of F3D, interactive rendering is not supported

This constitutes 2 distinct problems;

  1. F3D exists with 0 when clearly there is an error preventing normal operation
  2. F3D tries to default to interactive mode upon failing to read the json and subsequently fails since we are using the headless version which has no support for interactive mode.

To Reproduce
Steps to reproduce the behavior:

  1. Create an invalid json file on disk named test.json
  2. Run headless version of F3D (other versions may be affected as well) like so: f3d --config=test.json

Expected behavior
I expect the program to report the error and exit with a non-zero exit code such as 1

System Information:

  • Debian 11 x64 inside docker
  • F3D v2.2.1 headless
@Meakk
Copy link
Member

Meakk commented Dec 17, 2023

Probably related to #310
Are you using the occt plugin?

@mwestphal
Copy link
Contributor

mwestphal commented Dec 17, 2023

Please share your "myconfig.json" file

@mwestphal
Copy link
Contributor

mwestphal commented Dec 17, 2023

here is the file that was invalid for @mrdeveloperdude , but it is working well for me.

Test_files_F3D.zip

Lets consider that for this issue, the .json is just invalid and we should improve error management in this context

@snoyer
Copy link
Contributor

snoyer commented Dec 18, 2023

Original info from the Discord discussion:

Commandline looks like this:
/usr/bin/f3d --config= /tmp/1FGYnjiFIDanGgzNtCGOBT0bCfaO1fzdu_1702839294626.json /tmp/1FGYnjiFIDanGgzNtCGOBT0bCfaO1fzdu_1702839294626.stl
.stl file is present and should be valid
The error output produced by f3d looks like this:
"/tmp/1FGYnjiFIDanGgzNtCGOBT0bCfaO1fzdu_1702839294626.json" is not a file of a supported file format
This is a headless build of F3D, interactive rendering is not supported
(produced on stderr)

"foo.bar" is not a file of a supported file format is the message you get when opening an invalid scene/model/3d file.

The command /usr/bin/f3d --config= foo.json bar.stl has a space after --config= so config is actually "" and foo.json is part of the inputs. --config=foo.json (equal and no space) or --config foo.json (no equal) should work, assuming the json file is valid

Even though this is not a bug, possible improvements on F3D's side include:

  • maybe the model loading error message could be more specific? eg. "foo.bar" is not a file of a supported model/scene/3D file format, could not open 3D model/scene "foo.bar" (unsupported format), ...
  • --config="" (and equivalent --config= typo) should be invalid or at least give a warning (ideally that would mean using a proper optional type to mean "no value" internally instead of an empty string so the app can tell the difference. Or allow multiple --config arguments so empty vector is ok but empty/blank elements can be treated as invalid or at least suspicious)
  • maybe failing to load a model is fine in interactive mode but it should be an error in rendering to file mode? related question: what about multiple inputs when rendering to file, should that generate a warning when all but the first are ignored?

@mwestphal mwestphal added this to the 2.3.0 milestone Dec 21, 2023
@mwestphal mwestphal self-assigned this Dec 21, 2023
@mwestphal
Copy link
Contributor

@mwestphal
Copy link
Contributor

So I did not adress the incorrect syntax --config= path/to/conf.json. cxxops is unhelpfull so this will wait for #434

@mwestphal
Copy link
Contributor

All other point raised by @snoyer have been adressed.

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 a pull request may close this issue.

4 participants