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

raunak/add_debug_patch #45

Merged
merged 6 commits into from Mar 14, 2019

Conversation

Projects
None yet
5 participants
@raunakkathuria
Copy link
Contributor

raunakkathuria commented Mar 11, 2019

No description provided.

@@ -62,15 +64,33 @@ sub open_connection {

$c->on(text => sub {
my ($c, $msg) = @_;

my $original = "$msg";

This comment has been minimized.

@zakame

zakame Mar 13, 2019

Contributor

Is the string interpolation intentional?

This comment has been minimized.

@raunakkathuria

raunakkathuria Mar 13, 2019

Author Contributor

yes so as to be double sure that nothing modifies it

This comment has been minimized.

@binarytom

binarytom Mar 13, 2019

Contributor

@zakame We want to avoid any corruption caused by XS code or other unexpected behaviour - interpolating is only a partial guard against this, we're likely to hit CoW as well, so we may need '' . $msg or something more aggressive if this isn't enough.

This comment has been minimized.

@code4pay

code4pay Mar 13, 2019

would pay to add a comment with the above

$c->finish(1007 => 'Malformed JSON');
$log->debug(qq{JSON decoding failed for "$normalized_msg": $@});
}
my $decoded = eval { Encode::decode_utf8($msg) } or do {

This comment has been minimized.

@zakame

zakame Mar 13, 2019

Contributor

Might be worthwhile to compare with Encode::decode('UTF-8', $msg) as well

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 13, 2019

Codecov Report

Merging #45 into master will decrease coverage by 1.92%.
The diff coverage is 56.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   75.63%   73.71%   -1.93%     
==========================================
  Files           7        7              
  Lines         353      369      +16     
  Branches       58       60       +2     
==========================================
+ Hits          267      272       +5     
- Misses         48       57       +9     
- Partials       38       40       +2
Impacted Files Coverage Δ
lib/Mojo/WebSocketProxy/Dispatcher.pm 80.18% <56.52%> (-8.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae57b71...299f473. Read the comment docs.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 13, 2019

Codecov Report

Merging #45 into master will decrease coverage by 1.66%.
The diff coverage is 57.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   75.63%   73.97%   -1.67%     
==========================================
  Files           7        7              
  Lines         353      365      +12     
  Branches       58       60       +2     
==========================================
+ Hits          267      270       +3     
- Misses         48       55       +7     
- Partials       38       40       +2
Impacted Files Coverage Δ
lib/Mojo/WebSocketProxy/Dispatcher.pm 81.3% <57.89%> (-7.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae57b71...406f0d7. Read the comment docs.

@raunakkathuria raunakkathuria merged commit 6556b67 into binary-com:master Mar 14, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@raunakkathuria raunakkathuria deleted the raunakkathuria:raunak/add_debug_patch branch Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.