Discussion:
[Lift] Bug in the trait Troy
Vitaliy L
2018-08-28 10:49:25 UTC
Permalink
Hi Lift Commiters,

A current implementation of a tryo function has a few bugs. Basically,
there are two cases when the function throws an exception instead of
returning Failure. Here are these cases as a part of tests.

"return Failure(_, Full(NPE), _) in case of non empty ignore list without NPE" in {
val ignore: List[Class[_]] = List(classOf[IllegalArgumentException])

Box.tryo(ignore)(throw new NullPointerException) must beLike {
case Failure(_, Full(ex), _) => ex.getClass must_== classOf[NullPointerException]
}
}

"not throw NPE in case of nullable ignore list" in {
val ignore: List[Class[_]] = null

Box.tryo(ignore)(throw new IllegalArgumentException) must beLike {
case Failure(_, Full(ex), _) => ex.getClass must_== classOf[IllegalArgumentException]
}
}

The first one is about returning Failure with exception instead of throwing
exception in case of exception is not in nonempty ignore list.
The second one is about throwing NPE in case of nullable ignore list.
Basically, it is true for other parameters as well. For example for onError
param. But I left existing condition for checking ignore list for null.
Maybe is better to remove this condition at all or add the same checking to
onError parameter.


I have make a pull request https://github.com/lift/framework/pull/1952
already. Please review it.
--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Antonio Salazar Cardozo
2018-08-29 22:33:51 UTC
Permalink
Hey there Vitaliy, thanks for the PR! We'll have a look at it as soon as we
can and see about
getting it merged. Appreciate your taking the time to write a test and a
fix!
Thanks,
Antonio
Post by Vitaliy L
Hi Lift Commiters,
A current implementation of a tryo function has a few bugs. Basically,
there are two cases when the function throws an exception instead of
returning Failure. Here are these cases as a part of tests.
"return Failure(_, Full(NPE), _) in case of non empty ignore list without NPE" in {
val ignore: List[Class[_]] = List(classOf[IllegalArgumentException])
Box.tryo(ignore)(throw new NullPointerException) must beLike {
case Failure(_, Full(ex), _) => ex.getClass must_== classOf[NullPointerException]
}
}
"not throw NPE in case of nullable ignore list" in {
val ignore: List[Class[_]] = null
Box.tryo(ignore)(throw new IllegalArgumentException) must beLike {
case Failure(_, Full(ex), _) => ex.getClass must_== classOf[IllegalArgumentException]
}
}
The first one is about returning Failure with exception instead of
throwing exception in case of exception is not in nonempty ignore list.
The second one is about throwing NPE in case of nullable ignore list.
Basically, it is true for other parameters as well. For example for onError
param. But I left existing condition for checking ignore list for null.
Maybe is better to remove this condition at all or add the same checking to
onError parameter.
I have make a pull request https://github.com/lift/framework/pull/1952
already. Please review it.
--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Matt Farmer
2018-08-30 02:24:15 UTC
Permalink
Ditto to the above! I've reviewed and the changes look good to me. I'll
give others a day or so to weigh in before merging.

On Wed, Aug 29, 2018 at 6:33 PM Antonio Salazar Cardozo <
Post by Antonio Salazar Cardozo
Hey there Vitaliy, thanks for the PR! We'll have a look at it as soon as
we can and see about
getting it merged. Appreciate your taking the time to write a test and a
fix!
Thanks,
Antonio
Post by Vitaliy L
Hi Lift Commiters,
A current implementation of a tryo function has a few bugs. Basically,
there are two cases when the function throws an exception instead of
returning Failure. Here are these cases as a part of tests.
"return Failure(_, Full(NPE), _) in case of non empty ignore list without NPE" in {
val ignore: List[Class[_]] = List(classOf[IllegalArgumentException])
Box.tryo(ignore)(throw new NullPointerException) must beLike {
case Failure(_, Full(ex), _) => ex.getClass must_== classOf[NullPointerException]
}
}
"not throw NPE in case of nullable ignore list" in {
val ignore: List[Class[_]] = null
Box.tryo(ignore)(throw new IllegalArgumentException) must beLike {
case Failure(_, Full(ex), _) => ex.getClass must_== classOf[IllegalArgumentException]
}
}
The first one is about returning Failure with exception instead of
throwing exception in case of exception is not in nonempty ignore list.
The second one is about throwing NPE in case of nullable ignore list.
Basically, it is true for other parameters as well. For example for onError
param. But I left existing condition for checking ignore list for null.
Maybe is better to remove this condition at all or add the same checking to
onError parameter.
I have make a pull request https://github.com/lift/framework/pull/1952
already. Please review it.
--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
https://www.assembla.com/wiki/show/liftweb/Posting_example_code
---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an
For more options, visit https://groups.google.com/d/optout.
--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Loading...