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

fix: don't log when rawPacket is empty #2924

Merged
merged 11 commits into from
Aug 25, 2018
Merged

Conversation

popomore
Copy link
Member

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

@@ -91,6 +95,16 @@ describe('test/lib/cluster/app_worker.test.js', () => {
test2.request().path = '/foo bar';
await test2.expect(DEFAULT_BAD_REQUEST_HTML).expect(400);
});

it.only('should not log when there is no rawPacket', done => {
const socket = net.createConnection({ port: app.port }, () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

模拟不出来,等 @XadillaX 看看

@codecov
Copy link

codecov bot commented Aug 21, 2018

Codecov Report

Merging #2924 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2924      +/-   ##
==========================================
+ Coverage   99.75%   99.75%   +<.01%     
==========================================
  Files          29       29              
  Lines         825      826       +1     
==========================================
+ Hits          823      824       +1     
  Misses          2        2
Impacted Files Coverage Δ
lib/application.js 100% <100%> (ø) ⬆️

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 db1286d...d437659. Read the comment docs.

@fengmk2
Copy link
Member

fengmk2 commented Aug 21, 2018

其实 onClientError 异常不需要关注,互联网上大量无效请求,忽略就好。

@popomore
Copy link
Member Author

要区分的,之前就是遇到线上问题才打印出来的。

@fengmk2
Copy link
Member

fengmk2 commented Aug 21, 2018

我的意思是 onClientError 就不要打印异常日志了。

err.code,
err.message);
} else {
console.error(123123, '=============================');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

123?

module.exports = app => {
let server;
app.on('server', srv => { server = srv; });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好像默认已经挂到 app.server 了

@popomore popomore force-pushed the client-error-rawpacket branch from d986505 to 1b152e5 Compare August 24, 2018 05:02
@fengmk2
Copy link
Member

fengmk2 commented Aug 24, 2018

ci fail

@@ -62,13 +64,14 @@ describe('test/lib/cluster/app_worker.test.js', () => {
]);
});

describe('customized client error', () => {
describe.only('customized client error', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我在测这个 ci

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

travis 有一个 debug 模式,好像可以 ssh 上去搞

@popomore popomore merged commit 45e3024 into master Aug 25, 2018
@popomore popomore deleted the client-error-rawpacket branch August 25, 2018 12:33
popomore pushed a commit that referenced this pull request Aug 25, 2018
fix: don't log when rawPacket is empty (#2924)
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.

4 participants