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

Small modifications to PHP exception/error handling #4973

Merged
merged 3 commits into from
Apr 7, 2016

Conversation

mockey
Copy link
Contributor

@mockey mockey commented Mar 24, 2016

make HExceptions with an error code != null catchable, i.e. HExceptions from _hx_error_handler
remove duplicated error message, filename and line in HException from _hx_error_handler

make HExceptions with an error code != null catchable, i.e. HExceptions from _hx_error_handler
remove duplicated error message, filename and line from  _hx_error_handler
@mockey
Copy link
Contributor Author

mockey commented Mar 24, 2016

Some explainations (only if you're interested ;-):
There is this class HException, which extends the normal PHP Exception. It is needed as a wrapper so you can throw "anything" because PHP5 can only throw Exceptions. But the HException can never be caught as an Exception because of this hard coded line in genphp:
https://github.com/HaxeFoundation/haxe/blob/development/src/generators/genphp.ml#L1565
While this makes sense for things thrown from Haxe, it is not so good for HExceptions, that are thrown from _hx_error_handler, that covers error handling from PHP. So I added a check if the error code is set, which should only be set from the error handler.
Also I removed some duplicate text (message, file, line) from the HException that is thrown from _hx_error_handler.
This whole HException class should maybe be made private or something. It makes no sense to use it in Haxe code.

@Simn
Copy link
Member

Simn commented Mar 24, 2016

My first question is always if there's any way we can test this.

@mockey
Copy link
Contributor Author

mockey commented Mar 24, 2016

Exceptions can be tested in the test runner, I suppose? So when I catch an exception I can check its type and then proceed, right?

@Simn
Copy link
Member

Simn commented Mar 24, 2016

Yes, there should be an exc function which can be used to check exceptions. I don't know if that allows checking exception types though, so maybe just something custom would work best.

@mockey
Copy link
Contributor Author

mockey commented Mar 24, 2016

I'll try to make something up.

@Simn
Copy link
Member

Simn commented Apr 7, 2016

You'll have to change the test, it currently causes src/unit/TestIssues.hx:16: characters 30-39 : Module unit.issues.Issue4973 does not define type Issue4973 on all targets but PHP. Try #ifing out the test function instead.

@Simn Simn merged commit 019c78a into HaxeFoundation:development Apr 7, 2016
@mockey mockey deleted the php-exception-handling branch April 8, 2016 02:54
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