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

IO::Handle::close shouldn't decide what's a failure #6055

Closed
p6rt opened this issue Feb 4, 2017 · 15 comments
Closed

IO::Handle::close shouldn't decide what's a failure #6055

p6rt opened this issue Feb 4, 2017 · 15 comments

Comments

@p6rt
Copy link

@p6rt p6rt commented Feb 4, 2017

Migrated from rt.perl.org#130715 (status was 'rejected')

Searchable as RT130715$

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 4, 2017

From @briandfoy

While trying to work around #​125757 (using :out
makes the Proc object always return zero, I ran
into a different problem. I was writing test code
to check what a program does in cases where it
should exit with a non-zero status.

The docs for IO​::Handle​::close say only​:

  Will close a previously opened filehandle.

But other close methods mention using a LEAVE
phaser to avoid exceptions. I end up with kludgey
code like this to fight against Perl 6 thinking it
knows better than I do when something failed​:

  subtest {
  my $proc = run $*EXECUTABLE, $program, '-k', :err;
  my $message = try { $proc.err.slurp-rest };
  LEAVE { quietly { $proc.err.close } }

  like $message, rx​:i/invalid/, 'Invalid option warns';
  is $proc.exitcode, 2, 'exit code';
  }, "{$program} exits with false value with unknown switch";

Now, indeed this returned a non-zero exit status.
But, it did exactly what I wanted it to. This
isn't a failure in my code and it shouldn't be
exceptional.

Beyond that, there are many programs that use a
non-zero exit to mean something that isn't
failure. grep(1), for instance, uses 1 to mean no
lines matched. And so on. Perl 6 doesn't know
these, and I don't think it should make decisions
at this level.

I wouldn't mind the ability to throw an exception
if Proc was told to do that (say, like
:raise-exception similar to DBI's RaiseError).
But, it's easy enough for the programmer to make
this call on their own.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 4, 2017

From @zoffixznet

But other close methods mention using a LEAVE
phaser to avoid exceptions.

Would you be able to give a link where LEAVE is recommended?

$proc.err.close doesn't throw; it returns the Proc object. The throwage happens when a Proc with non-zero exit status is **sunk**.
So instead of the LEAVE stuff, the line can be written as `$proc.err.close.so;` or as `$ = $proc.err.close`, for example.

At the very least, this is a documentation issue.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 4, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @AlexDaniel

So after reading this wall of text I'm still confused. What is the actual problem that should be solved here?

For example, let's say you have something like this (dumb example, but still)​:

Code​:
shell ‘exit 1’

Result​:
The spawned command 'exit 1' exited unsuccessfully (exit code​: 1)
  in block <unit> at -e line 1

Last part of the ticket implies that we shouldn't blow up just because it has a non-zero exit code (“Perl 6 doesn't know these, and I don't think it should make decisions at this level”). In which case it can be rephrased to “here you have a safety feature that prevents silent failures, please remove it because I know better”. OK, but why are you running grep from perl 6 ?… If you really have to do that, then you can probably write an extra line of code to explicitly say that you expect non-zero exit codes.

The title, however, says that .close shouldn't throw. But the same logic applies here as well. If we make it not throw, then your code may have silent failures and you won't even notice. So handle it explicitly.

In other words, I think this should be rejected.

On 2017-02-03 18​:27​:34, comdog wrote​:

While trying to work around #​125757 (using :out
makes the Proc object always return zero, I ran
into a different problem. I was writing test code
to check what a program does in cases where it
should exit with a non-zero status.

The docs for IO​::Handle​::close say only​:

Will close a previously opened filehandle.

But other close methods mention using a LEAVE
phaser to avoid exceptions. I end up with kludgey
code like this to fight against Perl 6 thinking it
knows better than I do when something failed​:

subtest {
my $proc = run $*EXECUTABLE, $program, '-k', :err;
my $message = try { $proc.err.slurp-rest };
LEAVE { quietly { $proc.err.close } }

like $message, rx​:i/invalid/, 'Invalid option warns';
is $proc.exitcode, 2, 'exit code';
}, "{$program} exits with false value with unknown switch";

Now, indeed this returned a non-zero exit status.
But, it did exactly what I wanted it to. This
isn't a failure in my code and it shouldn't be
exceptional.

Beyond that, there are many programs that use a
non-zero exit to mean something that isn't
failure. grep(1), for instance, uses 1 to mean no
lines matched. And so on. Perl 6 doesn't know
these, and I don't think it should make decisions
at this level.

I wouldn't mind the ability to throw an exception
if Proc was told to do that (say, like
:raise-exception similar to DBI's RaiseError).
But, it's easy enough for the programmer to make
this call on their own.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @zoffixznet

I found where the LEAVE was mentioned and removed it.

The .close you're calling is actually from IO​::Pipe, not IO​::Handle.

Probably something else should be clarified in the docs to avoid that sort of confusion as well.

Marked it as TODO for my IO grant thing and will resolve this ticket as part of it.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @briandfoy

I shouldn't have to catch an exception for something doing exactly
what I want it to do. I don't think it's the language designer's place
to add why I might run grep from Perl 6, but the easy answer is
testing (as I showed in the original message).

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @AlexDaniel

so?

You do not have to catch the exception. Just don't sink your Proc. For example​:

shell(‘exit 1’).so

Of course, in actual code you'd have a proper check and no workaround would be needed​:

say ‘oops!’ unless shell(‘exit 1’)' # or whatever the normal exit code in this situation is

But given that your use case is “testing”, then a .so won't hurt.

On 2017-02-13 12​:46​:20, comdog wrote​:

I shouldn't have to catch an exception for something doing exactly
what I want it to do. I don't think it's the language designer's place
to add why I might run grep from Perl 6, but the easy answer is
testing (as I showed in the original message).

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @AlexDaniel

Please note that not checking the exit code is an error in almost all cases. For example, what if something you are running segfaults? Are you going to say that this is what you meant it to do?

On 2017-02-13 13​:05​:59, alex.jakimenko@​gmail.com wrote​:

so?

You do not have to catch the exception. Just don't sink your Proc. For
example​:

shell(‘exit 1’).so

Of course, in actual code you'd have a proper check and no workaround
would be
needed​:

say ‘oops!’ unless shell(‘exit 1’)' # or whatever the normal exit code
in this
situation is

But given that your use case is “testing”, then a .so won't hurt.

On 2017-02-13 12​:46​:20, comdog wrote​:

I shouldn't have to catch an exception for something doing exactly
what I want it to do. I don't think it's the language designer's
place
to add why I might run grep from Perl 6, but the easy answer is
testing (as I showed in the original message).

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @briandfoy

If something segfaults, that's a different issue (that I haven't
submitted yet(. The exit code shouldn't have a value at that point, I
don't think. If the program didn't exit, the Proc object shouldn't
have an exit code for it.

But, notice in the example I provided in this report, I am checking
the exit code. I'm not debating that.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @AlexDaniel

Well, in your particular example, I'd probably use something like​:

my $exit-code = $proc.err.close.exit-code;

And that'd be it.

Maybe you wanted something like this​:

$proc.err.slurp-rest(​:close)

Which, interestingly, hides the feature. But note that *this is not specced*.

On 2017-02-13 13​:24​:00, comdog wrote​:

If something segfaults, that's a different issue (that I haven't
submitted yet(. The exit code shouldn't have a value at that point, I
don't think. If the program didn't exit, the Proc object shouldn't
have an exit code for it.

But, notice in the example I provided in this report, I am checking
the exit code. I'm not debating that.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 14, 2017

From @raiph

This comment is technically redundant. And maybe not helpful. I apologize if it's annoying to anyone.

TL;DR; brian's right that built-ins shouldn't decide what's a failure in all but extremely exceptional circumstances (eg segfaults). Fortunately, they don't.

using a LEAVE phaser to avoid exceptions.

That's crazy. I was going to ask where that was in the docs but reading through this thread it looks like Zoffix found it and deleted it.

I end up with kludgey code like this to fight against Perl 6 thinking it knows better than I do when something failed

This is a doc problem, not Perl 6.

(Well, except bugs. If anything below doesn't work, there's a good chance it's a bug.)


Most Perl 6 routines are Socratic about apparent failures​: they know that they do NOT know what the caller knows.

Likewise Perl 6 itself; it generally gives callers of routines 100% control over how to interpret the results of any call.


Here's an attempt to explain this.

1. Every routine body must return a value. There are no exceptions. (Pun intended -- and meaningful​: in almost all cases for built-ins, at the point immediately after a value has been returned, no exception will have been raised even if a possibly catastrophic failure has occurred, precisely because it's up to the caller to make the final decision.)

2. A routine may return a Failure. This means the called routine wants the caller to decide what to do about something that *might* be a problem that needs attention.

3. Calling code must explicitly say when a possible problem, as signaled by a returned Failure, is not *actually* a problem (or at least that the calling code's got it covered). This avoids the problem of *accidentally* forgetting to handle real problems.

4. For example, if `some.expression` is in sink context (it's just a statement on its own) and its runtime value might be a Failure, and you know that this wouldn't be a problem, at least for now, then here are your main choices​:

4.1 `sink some.expression;`

In stark contrast to implicit sink context, explicitly sinking `some.expression` causes Perl 6 to silently throw away whatever value is returned, even a Failure.

4.2 `try some.expression;`

This stores any Failure in `$!`

4.3 `some.expression.so;`, `some.expression or True;` etc.

Testing a Failure with a boolean test or a defined test "disarms" it. (Though `so some.expression;` and `?some.expression;` don't work for some reason.)

5. Finally, if you do NOT *explicitly* `sink` a Failure that's in sink context to throw it away, and you do NOT `try` the expression that returned it to store problems in `$!`, and you do NOT disarm the Failure by testing it; if you do none of these things and then you use the value as an ordinary value, Perl 6 throws the til then unthrown Exception that sits inside the Failure.

Now, indeed this returned a non-zero exit status.
But, it did exactly what I wanted it to. This
isn't a failure in my code and it shouldn't be
exceptional.

Aiui, this only became an exception because your calling code didn't signal "I've got this" by explicitly `sink`ing or `try`ing or testing a routine's return value.

Beyond that, there are many programs that use a
non-zero exit to mean something that isn't
failure. grep(1), for instance, uses 1 to mean no
lines matched. And so on. Perl 6 doesn't know
these, and I don't think it should make decisions
at this level.

To recap, Perl 6 is not (supposed to be) making a final decision.

Even throwing an exception isn't a final decision -- calling code can handle exceptions.

But, much more importantly, Perl 6 waits until all other avenues are exhausted first.

Instead, Perl 6 makes it easy for called routines to return a value indicating something might be amiss and easy for calling code to handle that situation.

I wouldn't mind the ability to throw an exception
if Proc was told to do that (say, like
:raise-exception similar to DBI's RaiseError).

I think that's `use fatal;`. See https://docs.perl6.org/language/control#fail

But, it's easy enough for the programmer to make
this call on their own.

Yes. They just have to be explicit, eg `sink some.expression;` or `try some.expression` or `some.expression or ...` or `some.expression.so`, if they're calling code that sometimes returns a value indicating a possible error, eg a Failure.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 14, 2017

From @zoffixznet

On Mon, 13 Feb 2017 21​:04​:37 -0800, raiph wrote​:

This comment is technically redundant. And maybe not helpful. I
apologize if it's annoying to anyone.

Not annoying, but I feel we all have thoroughly confused bdfoy by now :P

Some minor corrections​:

4.1 `sink some.expression;`

In stark contrast to implicit sink context, explicitly sinking
`some.expression` causes Perl 6 to silently throw away whatever value
is returned, even a Failure.

That's inaccurate. `sink Failure.new` will explode the Failure, just
like sinking it implicitly would. The behaviour you describe is a bug
that was fixed in 2016.12 release.

4.2 `try some.expression;`

This stores any Failure in `$!`

That will store the exception the Failure contained in $!,
not the Failure itself.

4.3 `some.expression.so;`, `some.expression or True;` etc.
(Though `so some.expression;` and `?some.expression;` don't work for
some reason.)

Basically anything that ends up calling .Bool on Failure will mark it
as handled and make it non-explosive. Both `so` and `?`seem to work fine for me​:

  <ZoffixW> m​: say ?Failure.new
  <camelia> rakudo-moar b51a55​: OUTPUT«False␤»
  <ZoffixW> m​: say so Failure.new
  <camelia> rakudo-moar b51a55​: OUTPUT«False␤»

Aiui, this only became an exception because your calling code didn't
signal "I've got this" by explicitly `sink`ing or `try`ing or testing
a routine's return value.
[...]
I think that's `use fatal;`. See
https://docs.perl6.org/language/control#fail

There are actually no failures involved in the original code. The throwage
(not failurage) happens when the *Proc* object gets sunk. Tacking
on .so sinks the produced Bool instead.

This means `try $proc.err.close` won't stop the explosion, because
`try` receives the `Proc` object and returns it; it then gets
sunk and explodes.

Cheers,
ZZ

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 16, 2017

From @raiph

On Tue, 14 Feb 2017 11​:25​:44 -0800, cpan@​zoffix.com wrote​:

I feel we all have thoroughly confused bdfoy by now :P

Wow. I should probably not be writing another comment given how messed up my previous one was but for the record​:

The [sink] behaviour you describe is a bug that was fixed

That makes much more sense.

[failed trys] store the exception ... not the Failure

D'oh.

Both `so` and `?`seem to work fine for me

I must have been more tired than I realized. Now of course they work for me too. /o\

There are actually no failures involved in the original code. The throwage (not failurage) happens when the *Proc* object gets sunk.

Of course.

Thanks for cleaning up after me.

I want to say more. But in the hope that Brian won't actually think this all just proves he's right that Failures and/or Perl 6 sucks, I won't. ;)

@p6rt
Copy link
Author

@p6rt p6rt commented May 1, 2017

From @zoffixznet

On Fri, 03 Feb 2017 18​:27​:34 -0800, comdog wrote​:

While trying to work around #​125757 (using :out
makes the Proc object always return zero, I ran
into a different problem. I was writing test code
to check what a program does in cases where it
should exit with a non-zero status.

The docs for IO​::Handle​::close say only​:

Will close a previously opened filehandle\.

But other close methods mention using a LEAVE
phaser to avoid exceptions. I end up with kludgey
code like this to fight against Perl 6 thinking it
knows better than I do when something failed​:

subtest \{
    my $proc = run $\*EXECUTABLE, $program, '\-k', :err;
    my $message = try \{ $proc\.err\.slurp\-rest \};
    LEAVE \{ quietly \{ $proc\.err\.close  \} \}

    like $message, rx&#8203;:i/invalid/, 'Invalid option warns';
    is $proc\.exitcode, 2, 'exit code';
    \}, "\{$program\} exits with false value with unknown switch";

Now, indeed this returned a non-zero exit status.
But, it did exactly what I wanted it to. This
isn't a failure in my code and it shouldn't be
exceptional.

Beyond that, there are many programs that use a
non-zero exit to mean something that isn't
failure. grep(1), for instance, uses 1 to mean no
lines matched. And so on. Perl 6 doesn't know
these, and I don't think it should make decisions
at this level.

I wouldn't mind the ability to throw an exception
if Proc was told to do that (say, like
:raise-exception similar to DBI's RaiseError).
But, it's easy enough for the programmer to make
this call on their own.

Thank you for the report. However, I'm going to reject this ticket, as
the described issue is just a misunderstanding and does not exist​:

- "IO​::Handle​::close shouldn't decide what's a failure"
  - It doesn't. The explosion you show is due to a sunk Proc object.
- "The docs for IO​::Handle​::close say"
  - Those are the wrong docs. The code you show calls .close on IO​::Pipe.
  While IO​::Pipe is IO​::Handle, it provides its own close method[^1]
  that returns the Pipe's Proc object
- "But other close methods mention using a LEAVE phaser to avoid exceptions"
  - The now-reworded[^2] docs meant you could use LEAVE phaser to close
  a handle on scope leave, regardless of whether it's left normally or via
  a thrown exception (in the latter case, a .close at the end of the block
  wouldn't be reached). This isn't about *avoiding* any exceptions.
- "close shouldn't decide what's a failure"
  - It doesn't. Your .close returns the Proc object, which throws when sunk,
  if the process had non-zero status. There are no exceptions or failures
  involved in .close itself, so to avoid the explosion simply ensure the
  Proc doesn't get sunk
- "I end up with kludgey code"
  - You can close the Pipe via an arg given to .slurp (or .slurp-rest
  if you're on older Rakudos; will be deprecated in 6.d and removed in 6.e).
  And since the .close doesn't throw and we don't sink any Procs, no
  explosions happen and the code isn't kludgy​:

  use Test;
  my $program = ('-e', 'die "invalid"', '--');
  subtest {
  with run :err, $*EXECUTABLE, |$program, '-y' {
  like .err.slurp(​:close), rx​:i/invalid/, 'Invalid option warns';
  is .exitcode, 1, 'exit code';
  }
  }, "{$program} exits with false value with unknown switch";

[1] https://docs.perl6.org/type/IO::Pipe#method_close
[2] Raku/doc@2387ce3

-- IO grant

@p6rt
Copy link
Author

@p6rt p6rt commented May 1, 2017

@zoffixznet - Status changed from 'open' to 'rejected'

@p6rt p6rt closed this May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant