Skip to content

python: also catch general c++ exceptions#19440

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
paroj:pyexcept
Feb 3, 2021
Merged

python: also catch general c++ exceptions#19440
opencv-pushbot merged 1 commit intoopencv:3.4from
paroj:pyexcept

Conversation

@paroj
Copy link
Copy Markdown
Contributor

@paroj paroj commented Feb 2, 2021

they might be thrown from third-party code (notably Ogre in the ovis
module).
While Linux is kind enough to print them, they cause instant termination
on Windows.
Arguably, they do not origin from OpenCV itself, but still this helps
understanding what went wrong when calling an OpenCV function.

@andy-held

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • n/a There is reference to original bug report and related work
  • n/a There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=Custom
build_image:Custom=centos:7
buildworker:Custom=linux-1,linux-4,linux-6

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution!

Please rebase patch on 3.4 branch.

@paroj paroj changed the base branch from master to 3.4 February 2, 2021 13:02
@VadimLevin
Copy link
Copy Markdown
Contributor

VadimLevin commented Feb 2, 2021

@paroj thank you for contribution! Can you add a simple test for added scenario? It can be done as an additional method in the class Bindings located in the modules/python/test/test_misc.py.
For testing purpose you can add this simple function to modules/core/include/opencv2/core/bindings_utils.hpp

CV_WRAP static inline
void testRaiseGeneralException()
{
    throw std::exception("exception text");
}

and catch raised exception in Python using the following pattern:

with self.assertRaises((cv.error,),
                       msg='C++ exception is not propagated to Python in the right way') as cm:
    cv.utils.testRaiseGeneralException()
self.assertEqual(cm.exception.message, 'exception text')

Apart from that, maybe catch (...) statement should be added too, because C++ allows to throw anything?

@paroj paroj force-pushed the pyexcept branch 2 times, most recently from 1f5dc0f to bf88fb9 Compare February 2, 2021 13:42
@paroj
Copy link
Copy Markdown
Contributor Author

paroj commented Feb 2, 2021

@VadimLevin copy pasted the tests as desired. Are you aware that you can push to my PRs if you have maintainer access?

Apart from that, maybe catch (...) statement should be added too, because C++ allows to throw anything?

we would not be able to report anything useful in this case. Also ERRWRAP2 is added to every single function invocation in OpenCV and thus affects compile time - I already felt bad about making it longer. Therefore I would rather wait for evidence that this is needed..

@alalek
Copy link
Copy Markdown
Member

alalek commented Feb 2, 2021

catch(...)

ERRWRAP2() macro is not enough for that.
Perhaps this should be done on generator level to keep binding functions exception-free (not in scope of this PR). Possible error message is "Internal error in OpenCV Python bindings". It is much better that crash of Python runtime.

@paroj paroj force-pushed the pyexcept branch 3 times, most recently from 8413680 to 1b6f2e2 Compare February 2, 2021 18:16
they might be thrown from third-party code (notably Ogre in the ovis
module).
While Linux is kind enough to print them, they cause instant termination
on Windows.
Arguably, they do not origin from OpenCV itself, but still this helps
understanding what went wrong when calling an OpenCV function.
@opencv-pushbot opencv-pushbot merged commit 863ecde into opencv:3.4 Feb 3, 2021
@paroj paroj deleted the pyexcept branch February 3, 2021 12:05
@alalek alalek mentioned this pull request Feb 5, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants