Skip to content

Conversation

@ikirill
Copy link
Contributor

@ikirill ikirill commented Jan 22, 2021

It's not possible to rethrow outside a catch block (Julia gives an error). It was only checking for errors when ret was INVALID_ARGS which is wrong.

@ikirill
Copy link
Contributor Author

ikirill commented Jan 22, 2021

This doesn't really fix the issue because there is no backtrace saved with the exception so it's a little useless the way this pr is.

if e !== nothing && !isa(e, ForcedStop)
nlopt_exception = nothing
rethrow(e)
throw(e)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Too bad we lose the original stack trace; I guess in the future we could also potentially save the catch_backtrace() and wrap everything in our own exception type that shows this along with the original exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think wrapping the exception type might break try-catch blocks around the optimize call that expect specific exception types to be thrown.

I tried this: ikirill@c7aed00

@BioTurboNick
Copy link

I experienced this as well, would be really nice to just pass through the exception directly.

However, if a custom exception type is needed to provide additional information, callers would probably be fine catching that and then inspecting the inner exception.

What work is needed here?

@ikirill
Copy link
Contributor Author

ikirill commented Aug 5, 2022

Sorry I forgot about this PR. I think it works, I've been using my own local NLopt and it throws exceptions with backtraces correctly.

@ikirill
Copy link
Contributor Author

ikirill commented Aug 5, 2022

I also added saving backtraces to be shown with the exception. Obviously I can't rethrow the same exception, you have to save it from the function being called by C, and then throw it again on the Julia side once the C function returns. So maybe it's a little awkward.

@axsk
Copy link

axsk commented Dec 5, 2022

Just found this after haggling with debugging an error inside my objective without any stacktraces.
I hope it works fine and will get merged, thank you for the work @ikirill !

@gvnwst
Copy link

gvnwst commented Sep 11, 2023

@ikirill Any chance you've been working on this? It would be incredible to have this functionality.

@odow
Copy link
Member

odow commented Jan 15, 2024

Closing in favor of #194

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants