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

CALL/INFO fails silently #2635

Open
acook opened this issue Feb 12, 2025 · 7 comments
Open

CALL/INFO fails silently #2635

acook opened this issue Feb 12, 2025 · 7 comments

Comments

@acook
Copy link

acook commented Feb 12, 2025

Scenario 1

When:

Using CALL/INFO with an argument that calls and external program directly via string or block.

call/info "nonexistant"

Then:

The output from the REPL is the contents of a process information object with the ID of 0.

== make object! [
    id: 0
]

Scenario 2

When:

call/info "nonexistant" print ""

Then:

In this case even the strange output of Scenario 1 is masked by the following value. There is no output aside from the newline. This doesn't reveal any additional problems, but it shows how the problem may be masked and difficult to debug.

Expected Behavior

When not using /info, the error is printed as expected. This same error should be displayed in all cases.

** Access error: external process failed: "No such file or directory"
** Where: call
** Near: call "nonexistant"

I believe that removing the premature return here would fix this:

https://github.com/Oldes/Rebol3/blob/96b13b877805682b3eb9fefc7c870de355f85d8a/src/core/n-io.c#L903

Alternative

I could see that using /INFO would be a way to intentionally silence errors, but because the cause of the error is not contained in the information object there is no way for the program to log or report to the user what happened.

The early return was part of the original code, so this may have been the intended use case. Though it is possible that it was a mistake added in the Atronix additions but I didn't dig that far into the file history.

If this is the desired use then, then I might suggest that the error is captured in the information object rather than thrown out. Putting the string created by Make_OS_Error(r) into the object would supply the developer with the tools to properly handle the error without having to run a series of tests.

This code:

		if (r != 0) {
			REBCHR str[100];
			OS_FORM_ERROR(r, str, 100);

			val = Append_Frame(obj, NULL, SYM_ERROR);
			SET_STRING(val, Copy_OS_Str(str, (REBINT)LEN_STR(str)));
		}

Inserted here:

https://github.com/Oldes/Rebol3/blob/96b13b877805682b3eb9fefc7c870de355f85d8a/src/core/n-io.c#L901

Produces a result like:

== make object! [
    id: 0
    error: "No such file or directory"
]

I created a pull request with code that demonstrates this approach: Oldes/Rebol3#101

@Oldes
Copy link
Owner

Oldes commented Feb 12, 2025

I'm quite not sure with this one... actually I'm not much sure, why the /info was introduced, because it looks, it just returns a PID and the exit code only when used with /wait. I believe that there should be used a standard call object where would be always all fields. Because if the error field would be only in case of error, you would need additional check, if it is present.

@Oldes
Copy link
Owner

Oldes commented Feb 12, 2025

Also there is a question, if the object should not contain other fields... like used command, arguments, start time, end time and other possibly useful information.

@acook
Copy link
Author

acook commented Feb 12, 2025

I think more could be done with this, it is very under utilized.

I don't know where /INFO was supposed to be heading, but it is the only way to stop errors from automatically hitting the console currently, which feels a little rough all around. But I do prefer errors as values myself.

There are not really any other process management functions in Rebol, and CALL seems to bend over backwards to do many things. It is already one of the more complex builtins, judging by the docs.

Although, including exit-code when CALL isn't /WAITing would mean it would need to be set to NONE. I'm not sure if doing a null check is much better than a presence check, it just happens at a different stage in the process. Similar with the error field.

@Oldes
Copy link
Owner

Oldes commented Feb 12, 2025

Current call implementation comes from the Atronix branch. I think I never used the /info refinement. When implementing it now I would probably return a process handle instead of the pid as an integer when starting it asynchronously, so one could easily query all the info even from a running process whenever needed.

@Oldes
Copy link
Owner

Oldes commented Feb 12, 2025

When thinking about it, I would probably return the process handle only when the /info refinement would be used (instead of the current silly object). Because returning it when used call without any refinement would be too radical backward compatibility change.

The best advantage would be to be able monitor a state of the running process, like its memory/cpu usage. It would also allow automatic process termination.

Any ideas are welcome.

No timetable for it yet.. unfortunately.

@acook
Copy link
Author

acook commented Feb 12, 2025

Does Rebol already have the concept of a process handle? But something that can dynamically pull process info from the OS seems like the best idea compared to fully static objects.

@Oldes
Copy link
Owner

Oldes commented Feb 13, 2025

No.. there is not such a handle, but it may be added.

Originally handle was just a pointer (hidden integer), but now these may keep its type and also an internal context.

They are used for example to hold various cryptography contexts..
https://github.com/Oldes/Rebol3/blob/master/src/tests/units/dh-test.r3#L28-L29

I use these context handles mostly in extension... like here:
https://github.com/Oldes/Rebol-MiniAudio/blob/master/src/miniaudio-rebol-extension.c#L77-L120

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

No branches or pull requests

2 participants