Skip to content

Commit

Permalink
fix: onClientError remove content-length header (#3544)
Browse files Browse the repository at this point in the history
<!--
Thank you for your pull request. Please review below requirements.
Bug fixes and new features should include tests and possibly benchmarks.
Contributors guide: https://github.com/eggjs/egg/blob/master/CONTRIBUTING.md

感谢您贡献代码。请确认下列 checklist 的完成情况。
Bug 修复和新功能必须包含测试,必要时请附上性能测试。
Contributors guide: https://github.com/eggjs/egg/blob/master/CONTRIBUTING.md
-->

##### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [ ] `npm test` passes
- [ ] tests and/or benchmarks are included
- [ ] documentation is changed or added
- [ ] commit message follows commit guidelines

##### Affected core subsystem(s)
<!-- Provide affected core subsystem(s). -->


##### Description of change
<!-- Provide a description of the change below this comment. -->

提升测试覆盖率到 100%
  • Loading branch information
dead-horse authored and popomore committed Mar 16, 2019
1 parent c154b56 commit 04adb93
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 11 deletions.
14 changes: 7 additions & 7 deletions lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,28 +88,28 @@ class Application extends EggApplication {
}

[RESPONSE_RAW](socket, raw) {
/* istanbul ignore next */
if (!socket.writable) return;
if (!raw) return socket.end(DEFAULT_BAD_REQUEST_RESPONSE);

let body = (raw.body == null) ? DEFAULT_BAD_REQUEST_HTML : raw.body;
const body = (raw.body == null) ? DEFAULT_BAD_REQUEST_HTML : raw.body;
const headers = raw.headers || {};
const status = raw.status || 400;

let responseHeaderLines = '';
let lengthOutputed = false;
const firstLine = `HTTP/1.1 ${status} ${http.STATUS_CODES[status] || 'Unknown'}`;

// Not that safe because no validation for header keys.
// Refs: https://github.com/nodejs/node/blob/b38c81/lib/_http_outgoing.js#L451
for (const key of Object.keys(headers)) {
if (key.toLowerCase() === 'content-length') lengthOutputed = true;
if (key.toLowerCase() === 'content-length') {
delete headers[key];
continue;
}
responseHeaderLines += `${key}: ${escapeHeaderValue(headers[key])}\r\n`;
}

if (!lengthOutputed) {
if (!Buffer.isBuffer(body)) body = Buffer.from(body);
responseHeaderLines += `Content-Length: ${body.byteLength}\r\n`;
}
responseHeaderLines += `Content-Length: ${Buffer.byteLength(body)}\r\n`;

socket.end(`${firstLine}\r\n${responseHeaderLines}\r\n${body.toString()}`);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/core/messenger/local.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Messenger extends EventEmitter {
*/
sendTo(pid, action, data) {
debug('[%s] send %s with %j to %s', this.pid, action, data, pid);
if (pid !== process.pid) return;
if (pid !== process.pid) return this;
this.send(action, data, 'both');
return this;
}
Expand Down
78 changes: 78 additions & 0 deletions test/app/extend/context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,72 @@ describe('test/app/extend/context.test.js', () => {
});
});

describe('ctx.runInBackground(scope) with single process mode', () => {
// ctx.runInBackground with egg-mock are overrided
// single process mode will use the original ctx.runInBackground
let app;
before(async () => {
app = await utils.singleProcessApp('apps/ctx-background');
});
after(() => app.close());

it('should run background task success', async () => {
await app.httpRequest()
.get('/')
.expect(200)
.expect('hello');
await sleep(5000);
const logdir = app.config.logger.dir;
const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
assert(/background run result file size: \d+/.test(log));
assert(/background run anonymous result file size: \d+/.test(log));
assert(
/\[egg:background] task:saveUserInfo success \(\d+ms\)/.test(fs.readFileSync(path.join(logdir, 'egg-web.log'), 'utf8'))
);
assert(
/\[egg:background] task:.*?app[\/\\]controller[\/\\]home\.js:\d+:\d+ success \(\d+ms\)/.test(fs.readFileSync(path.join(logdir, 'egg-web.log'), 'utf8'))
);
});

it('should use custom task name first', async () => {
await app.httpRequest()
.get('/custom')
.expect(200)
.expect('hello');
await sleep(5000);
const logdir = app.config.logger.dir;
const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
assert(/background run result file size: \d+/.test(log));
assert(
/\[egg:background] task:customTaskName success \(\d+ms\)/.test(fs.readFileSync(path.join(logdir, 'egg-web.log'), 'utf8'))
);
});

it('should run background task error', async () => {
mm.consoleLevel('NONE');

let errorHadEmit = false;
app.on('error', (err, ctx) => {
assert(err.runInBackground);
assert(/ENOENT: no such file or directory/.test(err.message));
assert(ctx);
errorHadEmit = true;
});
await app.httpRequest()
.get('/error')
.expect(200)
.expect('hello error');
assert(errorHadEmit);
await sleep(5000);
const logdir = app.config.logger.dir;
const log = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
assert(/ENOENT: no such file or directory/.test(log));
assert(
/\[egg:background] task:mockError fail \(\d+ms\)/.test(fs.readFileSync(path.join(logdir, 'egg-web.log'), 'utf8'))
);
});
});

describe('tests on apps/demo', () => {
let app;
before(() => {
Expand Down Expand Up @@ -402,5 +468,17 @@ describe('test/app/extend/context.test.js', () => {
assert(ctx.getLogger('coreLogger') === logger);
});
});

describe('get router()', () => {
it('should alias to app.router', () => {
const ctx = app.mockContext();
assert(ctx.router === app.router);
});
it('should work with setter app.router', () => {
const ctx = app.mockContext();
ctx.router = 'router';
assert(ctx.router === 'router');
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ exports.onClientError = async (err, socket, app) => {

return {
body: err.rawPacket,
headers: { foo: 'bar' },
headers: { foo: 'bar', 'Content-Length': 100 },
status: 418,
};
};
5 changes: 4 additions & 1 deletion test/lib/cluster/app_worker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ describe('test/lib/cluster/app_worker.test.js', () => {
// customized client error response
const test1 = app.httpRequest().get('/foo bar');
test1.request().path = '/foo bar';
await test1.expect(html).expect('foo', 'bar').expect(418);
await test1.expect(html)
.expect('foo', 'bar')
.expect('content-length', '134')
.expect(418);

// customized client error handle function throws
const test2 = app.httpRequest().get('/foo bar');
Expand Down
19 changes: 18 additions & 1 deletion test/lib/core/messenger/local.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@ describe('test/lib/core/messenger/local.test.js', () => {
done();
});

app.messenger.sendTo(process.pid, 'sendTo-event', { foo: 'bar' });
let res = app.messenger.sendTo(process.pid, 'sendTo-event', { foo: 'bar' });
assert(res === app.messenger);
// should ignore if target process is not self
res = app.messenger.sendTo(1, 'sendTo-event', { foo: 'bar' });
assert(res === app.messenger);
});

it('agent.messenger.sendTo should work', done => {
Expand Down Expand Up @@ -196,4 +200,17 @@ describe('test/lib/core/messenger/local.test.js', () => {
app.agent.messenger.send('send-event', { foo: 'bar' });
});
});

describe('_onMessage()', () => {
it('should ignore if message format error', () => {
app.messenger._onMessage();
app.messenger._onMessage('foo');
app.messenger._onMessage({ action: 1 });
});

it('should emit with action', done => {
app.messenger.once('test-action', done);
app.messenger._onMessage({ action: 'test-action' });
});
});
});

0 comments on commit 04adb93

Please sign in to comment.