fatal warnings are a ticking time bomb (via chromatic)
chromatic just blogged about the perils of fatal warnings pushed down CPAN by the otherwise exceedingly useful Moo perl library. Since his comment system is perpetually broken, with his permission I am moving the discussion here, where the comment system is... right, anyhow.
What follows is the verbatim text of chromatic's post. My own thoughts are going to be in the form of a comment.
rest of text sourced from http://www.modernperlbooks.com/mt/2014/01/fatal-warnings-are-a-ticking-time-bomb.html
When Perl 5.6.0 introduced lexical warnings, it gave programmers finer control over what parts of their program would produce warnings and how to handle those warnings. The most important feature is that the warnings pragma has a lexical effect. In other words, rather than enabling warnings for the entire program unilaterally, some files and blocks can request stricter or looser warning categories than others.
Whole-Program Robustness is Not a Library Concern
This is especially important for Perl applications which are often a combination of custom code and CPAN modules. Even though the CPAN is free and open source software and even though many of the newer CPAN distributions have public source control repositories to which you can submit patches and feature requests, there's often a thick line between "code we maintain for this project" and "code someone else maintains". Lexical warnings mean that the maintainers of CPAN distributions can choose their own warning strategies and your team can choose your own warning strategy and those strategies don't have to be the same.
Keep that separation of concerns in mind for a moment.
Blurring the Lines Between Warnings and Exceptions
The warnings pragma also included a feature which can promote any warnings caused in a lexical scope into exceptions. If, for the purpose of security or clarity or maintainability or coding standards, your team decides that uninitialized value warnings are so severe that they should be exceptional conditions, you can ask the warnings pragma to promote that warning to an exception within a lexical scope. This is a powerful feature that you should use with caution; unless you're prepared to catch those exceptions and deal with them (or not catch them and deal with the consequences), those exceptions will cause your program to terminate. That may be what you want, but if you've asked for it explicitly, Perl believes you know what you're doing.
Perl also gives you the option to promote all warnings in every warning category to an exception with use warnings FATAL => 'all';
. If enabling fatal warnings for one warning category is serious, promoting all warnings to exceptions is grave.
Promoting Library Warnings to Exceptions
The warnings pragma also added an interesting feature by which modules can register their own categories of warnings. Perl will treat them as lexical warnings just as if they were core warnings. In other words, a module which chooses to register its own warning category will be able to emit warnings which respect a use warnings;
or no warnings;
in caller scopes.
This is a wonderful feature because it respects the separation of concerns. It's not up to a library to dictate what the users of that library consider warnable conditions. The users of that library can elect to accept warnings or disable them as they see fit.
Warnings registered with warnings also fall under the purview of the fatal warnings promotion code. If you've enabled fatal warnings and if any of the modules you use within that lexical scope emit a warning, that module's warning will become an exception.
Fatal Warnings are Not Future Proof
If you have the mindset for reading awkward bug reports, Perl RT #121085 demonstrates one serious danger of unilateral fatal warnings. Because you've asked Perl to treat all warnings as fatal, any warning you get will be fatal.
Because Perl, by default, does not treat warnings as fatal (because Perl uses the word "exception" to mean "exception" and "warning" to mean "warning", thus distinguishing between the two as a matter of syntax and semantics), it's possible that a newer major release of Perl may emit more warnings than a older major release of Perl. In fact, that happens. As Perl evolves and improves and Perl users discover more interesting and useful interactions with the language and as bugs get fixed, certain conditions lend themselves to error messages.
If you've written use warnings FATAL => 'all';
, you've accepted the responsibility for checking to see if a newer release of Perl emits newer warnings. Even if you didn't know you had that responsibility, you should have known. That's what you asked for. Your program threw an exception and you didn't catch it. That's your responsibility.
Many people know that. That fact is rarely controversial. If you read the RT #121085 bug report, you'll see the argument that p5p should be extra careful adding new warnings because some people choose to treat them as exceptions, and so they're effectively exceptions, and p5p is causing previously valid programs to break--but that's a silly argument.
Perl developers brag rightly about Perl's commitment to backwards compatibility, but an understated corollary to backwards compatibility is forwards compatibility. It's your responsibility as a developer to write code that's compatible with future releases of Perl (within reason) or to face the consequences. If you write code that is likely to break when upgrading to a new version of Perl, that's your fault. p5p will do its best to ease the transition (that's why deprecations are multi-year processes which begin with warnings and eventually become exceptions), but you have a responsibility too.
That fact is a little bit more controversial, but it's pretty well established on p5p. It's not at all well established on the CPAN, which is where the problem gets a lot more serious and subtle.
Remember how modules can register their own lexical warnings? Just as Perl may add new warnings in major releases, modules can add new warnings--except that CPAN modules can add new warnings whenever they see fit, because there are no normative community standards about when it's acceptable to add new warnings, how often releases will happen, how long the support period for releases is, and any commitment to backwards compatibility or deprecation.
If you've enabled fatal warnings, you've asked for any warnings in any CPAN module that you didn't write and you don't maintain to become exceptions which may terminate your program. The risk isn't "We upgrade our core Perl once a year and read the list of changes then run our entire test suite and only then deploy a new version". The risk is "We've updated CPAN modules and now have to audit all of them for new warnings."
That's a bigger risk--but it gets worse.
Fatal Warnings Do Not Belong in Module Code
What if a CPAN module you use but do not maintain uses fatal warnings? Any change to its dependencies which throws a warning the module did not account for may now throw an exception. Because it's in code you do not maintain, it's time to break out the debugger to figure out what went wrong, then fire off a patch and hope the maintainer releases a new version. In the meantime, your code is broken because a CPAN maintainer did not think about the risk of promoting warnings to exceptions for circumstances which are outside of his or her control.
"Wait," you say. "Which CPAN author would be silly enough to write use warnings FATAL => 'all';
in library code? That's awful, and I wouldn't use that code."
The problem is that Moo enables strictures by default in all code that simply writes use Moo;
and strictures
enables fatal warnings in all code which uses strictures
, so writing use Moo;
enables fatal warnings in the enclosing lexical scope.
In other words, any CPAN module which uses Moo
is, by default, vulnerable to a change in any of its dependencies which may legitimately produce new warnings. By "vulnerable" I mean "your program may start crashing in library code you didn't write and do not maintain and, by the way, may be in a dependency several levels deep that you didn't even know uses Moo
."
One solution is to use Moo::Lax instead of Moo
in code you write for the CPAN. That doesn't fix existing code, but it doesn't make the problem worse.
I suppose the well-intentioned people who wrote strictures
could make the argument that "You should be submitting patches against CPAN modules anyhow!", but I have trouble reconciling that against their justification for enabling fatal warnings, which seems to be "Novices should take exceptions seriously!" as the argument then becomes "If new Perl programmers aren't willing to learn enough about Perl to submit upstream patches when our code crashes their programs, there are many other languages they can use with library ecosystems which aren't hostile to new developers"--and that's a silly argument to make too. (Then again, strictures
has the misfeature, documented in RT #79504, that its behavior will change if you're in or slightly under a directory managed by a VCS even if you're not a Perl developer or even if it's not a Perl project or even if the program you're running has nothing to do with the directory or even if strictures
is a dependency of library code several levels deep.)
In Other Words, strictures
is Broken?
I suppose you could claim that it's Moo
that's broken, for inflicting the silliness that is the current behavior of strictures
on the CPAN and all downstream code that's unfortunate enough to somehow, somewhere use code that depends on Moo
, but the real problem is that use warnings FATAL => 'all';
does not belong in library code that you're handing other people to use. It's just too risky.
chromatic, I agree with you 100%, case in point: https://github.com/dbsrgits/dbix-class/commit/59d624cf78 There is also an ugly conflict coming down the pipes this year when I will have to merge Matt's amazing work on Data::Query into DBIx::Class stable, one of the blockers being his reliance on fatal warnings everywhere. Leaving them in would end up with the bizarre situation where the primary maints of DBIC can no longer guarantee its stability when exposed to DarkPan code.
There will need to be some sort of solution for this, and it will have to happen on a higher level than "use Moo::Lax". What I am slowly coming to realize is that the nature of fatal warnings being globalized by the auto-export, makes them impossible to tackle "per namespace". The problem will have to be tackled "per project". And I think there is a way forward (I was going to write a longwinded blogpost about this, but I might as well reuse your platform):
I think all parties (Matt included) would agree that fatal warnings as implemented in the language are unreliable. They kinda work in the 80/20 worse is better sense, but they can not be relied upon to fire all the time (#p5p folk is encouraged to chime in on the accuracy of this statement). As such any code that employs exception based flow control that hinges on a warning being promoted to an exception is broken by definition. Hence it should be safe to disable fatal warnings project-wide by loading an extra 'strictures::defang' or something similar early in your program, and having *all* callsites of 'use strictures' and 'use Moo' be in effect converted to a 'use warnings; use strict' with no extra mucking around.
When I dig myself from under a mountain of backlog next week, I was planning to address this issue as outlined above. Thanks for kicking off the discussion earlier.
Cheers
Another followup now that I examined the source of Moo::Lax. It in fact already sort-of does what I propose above, but does it in a very nasty silent way, without documenting it, and without checking whether strictures have already been used anywhere earlier. I consider this a problem and will be filing a bug to the effect.
A process-wide (app-wide) solution needs to be documented as such, and needs to ensure that it traps things early enough in the startup process.
@Peter Rabbitson : This is fixed in last release, thanks to leont. https://github.com/dams/p5-Moo-Lax/blob/master/lib/Moo/Lax.pm#L19
And another followup after I've had my proper amount of coffee: combining the late-night posting with the highly unusual "fire and replace yourself" implementation that Damien used, led me to misread his code altogether to look like global redifinition. Thus my "very nasty silent way" comment was entirely misplaced, and I fully retract it with apologies to Damien. Moreover the latest implementation on CPAN is much cleaner and robust thanks to Leon, so the misunderstanding ended up as a net-benefit.
In any case as chromatic pointed out earlier - this is a local solution which only goes half way. It still does not protect an unsuspecting CPAN user from a dependency which itself carelessly (or should I say arrogantly) uses fatal warnings internally. This is a problem.
I've just released a new version of Moops that tries to address this issue. Rather than enabling fatal warnings for all categories, it has a hard-coded list of warnings categories to enable it for. (This list is basically all the categories that were available in Perl 5.14, with four exceptions which I find annoying.)
Agreed! The lesson to take here is that whenever you do something with a global impact, you risk unintended side-effects. More than once I've found code altering the UNIVERSAL:: namespace in ways that have broken my apps.
How about not enabling fatal warnings in Moo in the first place by making strictures optional (and an optional dependency as well) and enabling strict and warnings only, just like the other members of the Moose family?
@gvl, that would be too easy. Also, isn't chromatic guilty of doing a similar type of thing by enabling UNIVERSAL::isa inside of Test::MockObject, which modifies core functionality globally? I remember that causing me a load trouble because of incompatibilities between UNIVERSAL::isa and upstream libraries that I had no control over.
Test::MockObject is fairly evil and can introduce hidden bugs -- its use of UNIVERSAL::isa and UNIVERSAL::can is just part of the problem. I'd recommend avoiding using these and just mock your object by hand as needed (it's very easy to use Class::Method::Modifiers to stub out the behaviour of a particular method for testing, or just override it directly with { 'no warnings "redefine"; sub ModuleToTest::some_method { }; }
I just noticed that the stub My:Module.pm created by Module::Starter has
Thanks! Bug filed: https://rt.cpan.org/Ticket/Display.html?id=93023