Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace error_get_last() with a custom error handler #334

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

j3j5
Copy link
Contributor

@j3j5 j3j5 commented Mar 18, 2022

Try to fix #332

This is my first serious attempt at a PR on here, so please, any suggestion is welcome.

While doing this I realized the implementation for the exception handling on the curl module was wrong. With PHP8 the $ch parameter doesn't exist anymore and it's now $handle. I fixed that and added the case for the other two new handles $multi_handle and $share_handle.

I've also started using the new function (PHP 8.0+) preg_last_error_msg() to determine the error message on PcreException.

PHPStan throws no errors on my machine but I didn't have time to test it properly, so more eyes are welcome before merging.

@Kharhamel
Copy link
Collaborator

I am sorry, I didn't have the time to review your PR, i will check it this week.

I also pinged @moufmouf who is much more expert in error_handlers

@j3j5 j3j5 force-pushed the global_error_handler branch from 4e98a78 to 847f5b6 Compare March 30, 2022 18:48
@j3j5
Copy link
Contributor Author

j3j5 commented Mar 30, 2022

I've rebased the branch and fixed the test. I'm updating the original description to mention the new PCRE Exception.

@j3j5 j3j5 force-pushed the global_error_handler branch from 847f5b6 to 6c2ad07 Compare April 13, 2022 22:23
@shish
Copy link
Collaborator

shish commented Feb 3, 2025

Reviving this project after a couple of years of inactivity, this PR looks like it might still be useful so I'm going to leave it open, but marking it as a draft until it is rebased on current master

@shish shish marked this pull request as draft February 3, 2025 12:36
@j3j5
Copy link
Contributor Author

j3j5 commented Feb 28, 2025

Given how long it has passed since I open this PR, I probably should just remake it instead of a simple rebase, I don't know why it's so big. I'll try to give it a try as soon as I have some free time.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take into account global error handlers
4 participants