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

Throw c++ exception when a Python exception occurs while running a algorithm #4764

Merged
merged 13 commits into from Jun 23, 2017

Conversation

nyalldawson
Copy link
Collaborator

This PR adds a number of building blocks required for catching python exceptions which occur in overriden virtual c++ methods and converting them to a c++ exception which is catchable in c++ code.

It then adds a new QgsProcessingException exception which is thrown when a python exception occurs while running a processing algorithm.

Copy link
Member

@wonder-sk wonder-sk left a comment

Choose a reason for hiding this comment

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

Looks good!

Would be nice though if we could

  • de-duplicate getTraceback() - e.g. to have a .h file that would be included both from core.sip and from src/python implementation
  • have a test that the exception is being thrown

Copy link
Member

@3nids 3nids left a comment

Choose a reason for hiding this comment

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

I can't comment much on the technical side here...
Anyway this sounds like a good catch!
Small details comments though.

* \ingroup core
* Custom exception class for processing related exceptions.
*/
class CORE_EXPORT QgsProcessingException : public QgsException
Copy link
Member

Choose a reason for hiding this comment

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

what about moving QgsException, QgsCsException and QgsProcessingException all to the same header?
they are almost the same and that would be coherent with the sip file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@@ -153,6 +153,11 @@
#define SIP_CONVERT_TO_SUBCLASS_CODE(code)

/*
* Virtual error handler (/VirtualErrorHandler/)
*/
#define SIP_VIRTUALERRORHANDLER(name)
Copy link
Member

Choose a reason for hiding this comment

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

please also add this to Doxyfile.in

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@nyalldawson
Copy link
Collaborator Author

de-duplicate getTraceback() - e.g. to have a .h file that would be included both from core.sip and from src/python implementation

Sorry - I failed here. Just couldn't get the linker happy.

have a test that the exception is being thrown

Done

@nirvn
Copy link
Contributor

nirvn commented Jun 23, 2017

Nice improvement.

@nyalldawson nyalldawson merged commit 19dd097 into qgis:master Jun 23, 2017
@nyalldawson nyalldawson deleted the processing_exception branch June 23, 2017 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants