Tests fail (with Protocol::WebSocket 0.22?)

[eserte]

Tests started to fail on my smokers:

        # Failed test at t/anyevent_websocket_connection.t line 154.
        # Unicode::GCString is not installed, table may not display all unicode characters properly
        # +---------------------+---------------------+----+----------+----------+
        # | PATH                | GOT                 | OP | CHECK    | LNs      |
        # +---------------------+---------------------+----+----------+----------+
        # |                     | AnyEvent::WebSocket |    | <OBJECT> | 150, 153 |
        # |                     | ::Connection=HASH(0 |    |          |          |
        # |                     | x44ca5cf0)          |    |          |          |
        # |                     |                     |    |          |          |
        # | close_code()        | 963                 | eq | 1005     | 440      |
        # | close_reason()      | �bb                 | eq | bb       | 440      |
        # +---------------------+---------------------+----+----------+----------+
    # Failed test 'close'
    # at t/anyevent_websocket_connection.t line 155.

(etc)

Statistical analysis suggests that the problem started with Protocol::WebSocket 0.22 (@vti @leonerd: FYI):

****************************************************************
Regression 'mod:Protocol::WebSocket'
****************************************************************
Name           	       Theta	      StdErr	 T-stat
[0='const']    	      1.0000	      0.0000	63928868152459800.00
[1='eq_0.21']  	      0.0000	      0.0000	   2.74
[2='eq_0.22']  	     -1.0000	      0.0000	-40146423047850984.00

R^2= 1.000, N= 135, K= 3
****************************************************************

[plicease]

Indeed, these changes are likely the trigger:
https://metacpan.org/diff/file?target=VTI/Protocol-WebSocket-0.22/lib%2FProtocol%2FWebSocket%2FFrame.pm&source=VTI%2FProtocol-WebSocket-0.21%2Flib%2FProtocol%2FWebSocket%2FFrame.pm

[vti]

Sorry, blame @leonerd for that :)

[leonerd]

Oops. This is almost certainly related to Unicode string vs. UTF-8 character handling in the surrounding code.

If someone can give me a bit of hand-holding about development/setup/testing on this code I'll be happy to take a look and see if I can fix it.

[leonerd]

Oh actually it's a ~2line patch to fix.

GH33.patch.txt

You may need to bump the version requirement of Protocol::WebSocket to the earliest one in which Protocol::WebSocket::Frame supports the opcode constructor argument - offhand I don't know which one that is. Maybe @vti can assist?

Clarification: It shouldn't need Protocol::WebSocket 0.22 itself here, as the change should be back-compat. The only issue was setting opcode before parsing the buffer, as it now needs to know if it's a text frame or not before it does that.

[plicease]

Thanks for the speedy fix. We already require Protocol::WebSocket 0.20, is that sufficient?

[plicease]

Patch seems to work with 0.20, 0.21, 0.22 and 0.23. Since the change was reverted in 0.23 it isn't necessary there (?), but it doesn't seem to do any harm either.

[vti]

It effected other modules that's why I reverted it for now.

[plicease]

yeah. I gathered.

I've released this fix as WSC 0.44, as it fixes compat with PW 0.22 and doesn't seem to break any of the other versions that we support. Please let me know if there are any changes in WSC required for good fix referred to here: vti/protocol-websocket#54 (comment)

[vti]

I will try to make sure dependent modules keep working %)

…

On Dec 12, 2017 21:43, "✈ Graham ✈" ***@***.***> wrote: yeah. I gathered. I've released this fix as WSC 0.44, as it fixes compat with PW 0.22 and doesn't seem to break any of the other versions that we support. Please let me know if there are any changes in WSC required for good fix referred to here: vti/protocol-websocket#54 (comment) <vti/protocol-websocket#54 (comment)> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#33 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB6M4tEvilnGTOMYGBAfhFDkouUCsHNks5s_uV4gaJpZM4Q8Zzr> .