Skip to content

Catch exceptions from TraceRay filters/enumerators#1557

Merged
asherkin merged 1 commit intomasterfrom
trerror
Aug 2, 2021
Merged

Catch exceptions from TraceRay filters/enumerators#1557
asherkin merged 1 commit intomasterfrom
trerror

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