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

only close curl handle if it's done. #2950

Merged
merged 1 commit into from Dec 6, 2021

Conversation

divinity76
Copy link
Contributor

@divinity76 divinity76 commented Oct 19, 2021

this is a copy of PR #2892

quoting bagder:

Can I object to the closing of this PR? I'm the main author of libcurl and I wrote this API. It is clearly documented to return a CURLMSG_DONE for this case and only then does it mean that a handle/transfer is complete. Not checking for that value works only by chance and will break in the case libcurl adds another message.

If you're not going to listen to badger, the guy who wrote libcurl, then there's no hope.

that said, another option would be to throw a LogicException instead, something along the line of

            if ($done['msg'] !== \CURLMSG_DONE) {
                throw new \LogicException("unknown message received from libcurl. only CURLMSG_DONE is supported. received ".$msg['msg'] );
            }

but.. just ignoring unknown messages is the least destructive approach.

@GrahamCampbell GrahamCampbell merged commit e6765c0 into guzzle:master Dec 6, 2021
@divinity76
Copy link
Contributor Author

Nice to see it fixed. While we're on the subject, i wonder if a E_USER_WARNING about unknown-ignored message would be appropriate, but won't waste much more time thinking about it

@GrahamCampbell
Copy link
Member

Thanks @divinity76. 🍻

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.

None yet

2 participants