Skip to content

Commit

Permalink
fix(2068): Large step logs causes browser to hang (#2200)
Browse files Browse the repository at this point in the history
* fix(2068): Large step logs causes browser to hang

* fix uuid

* fix uuid

* fix test

* add comment

Co-authored-by: Kevin Lu <[email protected]>
  • Loading branch information
klu909 and Kevin Lu authored Oct 6, 2020
1 parent 49391f7 commit 0faaaa9
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 11 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@
"@hapi/good-console": "^9.0.0",
"@hapi/good-squeeze": "^6.0.0",
"@hapi/hapi": "^20.0.0",
"@hapi/hoek": "^9.0.4",
"@hapi/hoek": "^9.1.0",
"@hapi/inert": "^6.0.1",
"@hapi/vision": "^6.0.0",
"@promster/hapi": "^4.2.0",
"async": "^3.2.0",
"config": "^1.31.0",
"date-fns": "^1.30.1",
"dayjs": "^1.8.33",
"dayjs": "^1.9.1",
"deepmerge": "^4.2.2",
"hapi-auth-bearer-token": "^6.1.6",
"hapi-auth-jwt2": "^10.1.0",
Expand Down
50 changes: 41 additions & 9 deletions plugins/builds/steps/logs.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const request = require('request');
const ndjson = require('ndjson');
const MAX_LINES_SMALL = 100;
const MAX_LINES_BIG = 1000;
const jwt = require('jsonwebtoken');
const uuid = require('uuid');

const logger = require('screwdriver-logger');

Expand Down Expand Up @@ -144,7 +146,7 @@ module.exports = config => ({
notes: 'Returns the logs for a step',
tags: ['api', 'builds', 'steps', 'log'],
auth: {
strategies: ['token'],
strategies: ['token', 'session'],
scope: ['user', 'pipeline', 'build']
},
plugins: {
Expand All @@ -156,7 +158,6 @@ module.exports = config => ({
const { stepFactory } = req.server.app;
const buildId = req.params.id;
const stepName = req.params.name;
const { headers } = req;

return stepFactory
.get({ buildId, name: stepName })
Expand All @@ -174,10 +175,28 @@ module.exports = config => ({

const isDone = stepModel.code !== undefined;
const baseUrl = `${config.ecosystem.store}/v1/builds/${buildId}/${stepName}/log`;
const authToken = headers.authorization;
const { sort } = req.query;
const pagesToLoad = req.query.pages;
const linesFrom = req.query.from;
const authToken = jwt.sign(
{
buildId,
stepName,
scope: ['user']
},
config.authConfig.jwtPrivateKey,
{
algorithm: 'RS256',
expiresIn: '5s',
jwtid: uuid.v4()
}
);
const { sort, type } = req.query;
let pagesToLoad = req.query.pages;
let linesFrom = req.query.from;

if (type === 'download' && isDone) {
// 100 lines per page
pagesToLoad = Math.ceil(stepModel.lines / 100);
linesFrom = 0;
}

// eslint-disable-next-line max-len
return getMaxLines({ baseUrl, authToken })
Expand All @@ -191,9 +210,22 @@ module.exports = config => ({
maxLines
})
)
.then(([lines, morePages]) =>
h.response(lines).header('X-More-Data', (morePages || !isDone).toString())
);
.then(([lines, morePages]) => {
if (type !== 'download') {
return h.response(lines).header('X-More-Data', (morePages || !isDone).toString());
}

let res = '';

for (let i = 0; i < lines.length; i += 1) {
res = `${res}${lines[i].m}\n`;
}

return h
.response(res)
.type('text/plain')
.header('content-disposition', `attachment; filename="${stepName}-log.txt"`);
});
})
.catch(err => {
throw err;
Expand Down
25 changes: 25 additions & 0 deletions test/plugins/builds.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4525,6 +4525,31 @@ describe('build plugin test', () => {
});
});

it('returns download link for download option', () => {
nock('https://store.screwdriver.cd')
.get(`/v1/builds/${id}/${step}/log.0`)
.twice()
.replyWithFile(200, `${__dirname}/data/step.log.ndjson`);

const expectedLog = 'Building stuff\nStill building...\nDone Building stuff\n';

return server
.inject({
url: `/builds/${id}/steps/${step}/logs?type=download`,
auth: {
credentials: {
scope: ['user']
},
strategy: ['session']
}
})
.then(reply => {
assert.equal(reply.statusCode, 200);
assert.deepEqual(reply.result, expectedLog);
assert.propertyVal(reply.headers, 'content-disposition', `attachment; filename="${step}-log.txt"`);
});
});

it('returns logs for a step that is split across pages', () => {
nock('https://store.screwdriver.cd')
.get(`/v1/builds/${id}/${step}/log.0`)
Expand Down

0 comments on commit 0faaaa9

Please sign in to comment.