Skip to content

Commit 0faaaa9

Browse files
klu909Kevin Lu
and
Kevin Lu
authoredOct 6, 2020
fix(2068): Large step logs causes browser to hang (#2200)
* fix(2068): Large step logs causes browser to hang * fix uuid * fix uuid * fix test * add comment Co-authored-by: Kevin Lu <[email protected]>
1 parent 49391f7 commit 0faaaa9

File tree

3 files changed

+68
-11
lines changed

3 files changed

+68
-11
lines changed
 

‎package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,14 @@
7474
"@hapi/good-console": "^9.0.0",
7575
"@hapi/good-squeeze": "^6.0.0",
7676
"@hapi/hapi": "^20.0.0",
77-
"@hapi/hoek": "^9.0.4",
77+
"@hapi/hoek": "^9.1.0",
7878
"@hapi/inert": "^6.0.1",
7979
"@hapi/vision": "^6.0.0",
8080
"@promster/hapi": "^4.2.0",
8181
"async": "^3.2.0",
8282
"config": "^1.31.0",
8383
"date-fns": "^1.30.1",
84-
"dayjs": "^1.8.33",
84+
"dayjs": "^1.9.1",
8585
"deepmerge": "^4.2.2",
8686
"hapi-auth-bearer-token": "^6.1.6",
8787
"hapi-auth-jwt2": "^10.1.0",

‎plugins/builds/steps/logs.js

+41-9
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ const request = require('request');
66
const ndjson = require('ndjson');
77
const MAX_LINES_SMALL = 100;
88
const MAX_LINES_BIG = 1000;
9+
const jwt = require('jsonwebtoken');
10+
const uuid = require('uuid');
911

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

@@ -144,7 +146,7 @@ module.exports = config => ({
144146
notes: 'Returns the logs for a step',
145147
tags: ['api', 'builds', 'steps', 'log'],
146148
auth: {
147-
strategies: ['token'],
149+
strategies: ['token', 'session'],
148150
scope: ['user', 'pipeline', 'build']
149151
},
150152
plugins: {
@@ -156,7 +158,6 @@ module.exports = config => ({
156158
const { stepFactory } = req.server.app;
157159
const buildId = req.params.id;
158160
const stepName = req.params.name;
159-
const { headers } = req;
160161

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

175176
const isDone = stepModel.code !== undefined;
176177
const baseUrl = `${config.ecosystem.store}/v1/builds/${buildId}/${stepName}/log`;
177-
const authToken = headers.authorization;
178-
const { sort } = req.query;
179-
const pagesToLoad = req.query.pages;
180-
const linesFrom = req.query.from;
178+
const authToken = jwt.sign(
179+
{
180+
buildId,
181+
stepName,
182+
scope: ['user']
183+
},
184+
config.authConfig.jwtPrivateKey,
185+
{
186+
algorithm: 'RS256',
187+
expiresIn: '5s',
188+
jwtid: uuid.v4()
189+
}
190+
);
191+
const { sort, type } = req.query;
192+
let pagesToLoad = req.query.pages;
193+
let linesFrom = req.query.from;
194+
195+
if (type === 'download' && isDone) {
196+
// 100 lines per page
197+
pagesToLoad = Math.ceil(stepModel.lines / 100);
198+
linesFrom = 0;
199+
}
181200

182201
// eslint-disable-next-line max-len
183202
return getMaxLines({ baseUrl, authToken })
@@ -191,9 +210,22 @@ module.exports = config => ({
191210
maxLines
192211
})
193212
)
194-
.then(([lines, morePages]) =>
195-
h.response(lines).header('X-More-Data', (morePages || !isDone).toString())
196-
);
213+
.then(([lines, morePages]) => {
214+
if (type !== 'download') {
215+
return h.response(lines).header('X-More-Data', (morePages || !isDone).toString());
216+
}
217+
218+
let res = '';
219+
220+
for (let i = 0; i < lines.length; i += 1) {
221+
res = `${res}${lines[i].m}\n`;
222+
}
223+
224+
return h
225+
.response(res)
226+
.type('text/plain')
227+
.header('content-disposition', `attachment; filename="${stepName}-log.txt"`);
228+
});
197229
})
198230
.catch(err => {
199231
throw err;

‎test/plugins/builds.test.js

+25
Original file line numberDiff line numberDiff line change
@@ -4525,6 +4525,31 @@ describe('build plugin test', () => {
45254525
});
45264526
});
45274527

4528+
it('returns download link for download option', () => {
4529+
nock('https://store.screwdriver.cd')
4530+
.get(`/v1/builds/${id}/${step}/log.0`)
4531+
.twice()
4532+
.replyWithFile(200, `${__dirname}/data/step.log.ndjson`);
4533+
4534+
const expectedLog = 'Building stuff\nStill building...\nDone Building stuff\n';
4535+
4536+
return server
4537+
.inject({
4538+
url: `/builds/${id}/steps/${step}/logs?type=download`,
4539+
auth: {
4540+
credentials: {
4541+
scope: ['user']
4542+
},
4543+
strategy: ['session']
4544+
}
4545+
})
4546+
.then(reply => {
4547+
assert.equal(reply.statusCode, 200);
4548+
assert.deepEqual(reply.result, expectedLog);
4549+
assert.propertyVal(reply.headers, 'content-disposition', `attachment; filename="${step}-log.txt"`);
4550+
});
4551+
});
4552+
45284553
it('returns logs for a step that is split across pages', () => {
45294554
nock('https://store.screwdriver.cd')
45304555
.get(`/v1/builds/${id}/${step}/log.0`)

0 commit comments

Comments
 (0)
Please sign in to comment.