Currently, there is no explanation of why a WebSocket connection is not established - all a developer gets is a "close" event. While this is correct behavior for Web content, it's extremely unfriendly to developers.
One particularly mysterious case that we need to report is failing to establish a connection due to SSL failures. For CFNetwork, we can use APIs such as CFErrorCopyDescription() to report the errors.
I will take care of this. Here is rough sketch of my implementation plan: * Add some members (failingURL and localizedDescription for example, as in ResourceError) to SocketStreamError. * Pass the reason of a failure to SocketStreamError instance (in each port), and notify it via SocketStreamHandleClient::didFail(). * Log the reason of failure in WebSocketChannel::didFail handler.
Created attachment 76866 [details] Patch
Comment on attachment 76866 [details] Patch +ws://nonexistent.domain.invalid:80/WebSocket network error: error code 2 (Operation could not be completed. (kCFErrorDomainCFNetwork error 2.)) It isn't great to print the almost useless error code twice. This bug is originally about invalid SSL certificates. Did you check that a useful message is printed for those? How does it look? +#ifdef BUILDING_ON_TIGER Please put modern code first, and Tiger specific in a fallback branch (i.e. please reverse both ifdefs). This is definitely an improvement, r=me. Please do check that this addresses the original issue reported here (printing an error for SSL certificates).
(In reply to comment #4) > (From update of attachment 76866 [details]) > +ws://nonexistent.domain.invalid:80/WebSocket network error: error code 2 (Operation could not be completed. (kCFErrorDomainCFNetwork error 2.)) > > It isn't great to print the almost useless error code twice. I will change the code so that it will not show errorCode when localizedDescription is provided. > > This bug is originally about invalid SSL certificates. Did you check that a useful message is printed for those? How does it look? I tried, and it looked like: >WebSocket network error: error code -9812 (Operation could not be completed. (OSStatus Error -9812)) ...which sounds less helpful. It looks like this status code comes from SecureTransport.h: >errSSLUnknownRootCert = -9812, /* valid cert chain, untrusted root */ Do you have any idea about how to improve this message? I do not have a good experience on Mac programming, and do not know a portable way to convert this status code into human-readable description. > > +#ifdef BUILDING_ON_TIGER > > Please put modern code first, and Tiger specific in a fallback branch (i.e. please reverse both ifdefs). Sure. > > This is definitely an improvement, r=me. Please do check that this addresses the original issue reported here (printing an error for SSL certificates). Thank you for your review!
On Windows, the error message about SSL certificate will look like: > WebSocket network error: error code -2146762487 (The operation couldn’t be completed. (OSStatus error -2146762487.)) I have no idea about where this value comes from...
For errors in kCFErrorDomainOSStatus domain, GetMacOSStatusCommentString() might print a useful message. I'd be slightly surprised if that worked when CFErrorCopyDescription() didn't, but seems worth a try.
Oops, I just noticed a memory leak: + RetainPtr<CFErrorRef> error(CFReadStreamCopyError(m_readStream.get())); This should be (in two places): + RetainPtr<CFErrorRef> error(AdoptCF, CFReadStreamCopyError(m_readStream.get()));
And same for description, of course - CFErrorCopyDescription is also a "copy" function.
Created attachment 76994 [details] Patch v2
I'm requesting a review again because I made significant changes to the previous patch: - Changed error handling in SocketStreamHandle to handle OSStatus - Added a test to check invalid SSL certificate.
Comment on attachment 76994 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=76994&action=review > WebCore/platform/network/cf/SocketStreamHandleCFNet.cpp:599 > +void SocketStreamHandle::notifyCFErrorToClient(CFErrorRef error) The name of this function is not grammatically correct. I'd say "reportErrorToClient" or just "reportError" - there is no need to encode argument type in funticon name.
Committed r74390: <http://trac.webkit.org/changeset/74390>
I've filed <rdar://problem/8787916> about making CFErrorCopyDescription use a better string for Security framework's OSStatus errors.