Skip to content

Catch exceptions from TraceRay filters/enumerators #1557

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

Merged
merged 1 commit into from
Aug 2, 2021
Merged

Conversation

asherkin
Copy link
Member

When a custom TraceRay filter / EnumerateEntities enumerator callback
throws an exception we currently continue execution and then return
execution to the calling code as if there were no problems. This
currently causes a heap tracking issue in SourcePawn, but even ignoring
that it is likely the wrong behaviour and differs from our other
synchronous callbacks.

This change causes the exception to be caught, immediately terminates
the trace / enumeration, and propagates the exception state back to the
calling plugin correctly. The implementation here is based on how
SortCustom1D handles exceptions.

See also alliedmodders/sourcepawn#607. Thanks again to @dysphie for the report.

When a custom TraceRay filter / EnumerateEntities enumerator callback
throws an exception we currently continue execution and then return
execution to the calling code as if there were no problems. This
currently causes a heap tracking issue in SourcePawn, but even ignoring
that it is likely the wrong behaviour and differs from our other
synchronous callbacks.

This change causes the exception to be caught, immediately terminates
the trace / enumeration, and propagates the exception state back to the
calling plugin correctly. The implementation here is based on how
SortCustom1D handles exceptions.
@asherkin asherkin requested a review from dvander July 31, 2021 01:21
@KyleSanderson
Copy link
Member

I'm not sure we want this. Each plugin callback is fired individually, so it enters the function, has an error, it should continue firing until done, no? I could go either way, though.

@asherkin
Copy link
Member Author

asherkin commented Aug 1, 2021

Unfortunately in both cases if the callback has errored at all the trace/enumeration probably hasn't done what was intended - a big part of the change here is that now the error propagates back up so that execution doesn't continue as the state is bad, so it doesn't make sense to keep calling the callback.

@asherkin asherkin merged commit 3c79701 into master Aug 2, 2021
@asherkin asherkin deleted the trerror branch August 2, 2021 10:57
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.

2 participants