Protect DBIC as best we can from the failure mode in 7cb35852
authorPeter Rabbitson <ribasushi@cpan.org>
Wed, 3 Feb 2016 00:32:00 +0000 (01:32 +0100)
committerPeter Rabbitson <ribasushi@cpan.org>
Wed, 3 Feb 2016 18:40:18 +0000 (19:40 +0100)
commitddcc02d14d03169c54c65db9f0f446836483ba55
tree51b1b3299e68c6f0c8fab82a70d9db64c02d7fdb
parentdb83437ef48f4571e1d225572cc7235eb5e64fe3
Protect DBIC as best we can from the failure mode in 7cb35852

The main idea is that while exception_action works just like $SIG{__DIE__},
this is less than ideal as non-sufficiently-careful software can completely
abort a callstack by goto()-ing out of it (not to mention the annoyance of
users when receiving callbacks on exceptions that DBIC later handles).

So this changeset makes exception_action() behave *better* than $SIG{__DIE__}
by meticlously annotating every DBIC-internal recoverable-exception site and
ensuring that exception_action (and any $SIG{__DIE__} callback) is not
invoked in this case ( see the diff of t/61findnot.t specifically )

This is a rather heavy and involved set of changes, but there seems to be no
other way to go around this. There were complaints already due to firing o
handlers on recoverable errors, but this is the first time the integrity of
the actul DBIC code flow was broken. Thus an executive decision was made to
solve this for good (took about 2 full days of work, sigh...)

The main part of this changset is in ::Schema.pm and ::_Util.pm, the rest is
simply switching from try/eval =>  dbic_internal_try. Some codepaths can not
be executed due to lack of RDBMS, but afaict it all works.

The changes were audited by a combination of:

  watch -x grep -rnP --exclude='*.pod' '^(?!\s*\#).*?(\beval\b|\btry\b)' lib

  git diff HEAD^ \
  |  perl -0777 -e 'my $str = <>; while( $str =~ /(?:\A|^index)(.+?)(?:^diff|\z)/gsm) { my $substr = $1; warn $substr if ( $substr =~ /dbic_internal_try/ and $substr !~ /DBIx::Class::_Util/ ) }' 2>&1 \
  | less

And a BaseSchema.pm exception_action hook which registers failures on any
invocation which is not paired with an eval/thows_ok/etc in a t/**.t frame.
In other words we make sure when exception_action is invoked - a test is there
waiting for the resulting exception, assuming any other exception is transient
( needs DBICTEST_ASSERT_NO_SPURIOUS_EXCEPTION_ACTION to be set ).

Additionally there is a global CI-enabled Try::Tiny::try override which fails
on calls from within the DBIx::Class namespace.
38 files changed:
Changes
lib/DBIx/Class/Carp.pm
lib/DBIx/Class/InflateColumn/DateTime.pm
lib/DBIx/Class/Relationship/BelongsTo.pm
lib/DBIx/Class/Relationship/CascadeActions.pm
lib/DBIx/Class/Relationship/HasMany.pm
lib/DBIx/Class/Relationship/HasOne.pm
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/ResultSource.pm
lib/DBIx/Class/ResultSourceHandle.pm
lib/DBIx/Class/Row.pm
lib/DBIx/Class/Schema.pm
lib/DBIx/Class/Schema/Versioned.pm
lib/DBIx/Class/Storage.pm
lib/DBIx/Class/Storage/BlockRunner.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/Cursor.pm
lib/DBIx/Class/Storage/DBI/InterBase.pm
lib/DBIx/Class/Storage/DBI/MSSQL.pm
lib/DBIx/Class/Storage/DBI/ODBC/Firebird.pm
lib/DBIx/Class/Storage/DBI/ODBC/Microsoft_SQL_Server.pm
lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
lib/DBIx/Class/Storage/DBI/Replicated.pm
lib/DBIx/Class/Storage/DBI/Replicated/Pool.pm
lib/DBIx/Class/Storage/DBI/Replicated/WithDSN.pm
lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm
lib/DBIx/Class/Storage/DBI/SQLite.pm
lib/DBIx/Class/Storage/DBI/Sybase.pm
lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
lib/DBIx/Class/Storage/DBI/Sybase/FreeTDS.pm
lib/DBIx/Class/_Util.pm
lib/SQL/Translator/Parser/DBIx/Class.pm
maint/travis-ci_scripts/20_install.bash
t/34exception_action.t
t/61findnot.t
t/lib/DBICTest/BaseSchema.pm
t/lib/DBICTest/RunMode.pm
t/storage/reconnect.t