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.

Sign up for GitHub

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

Jump to bottom

Don't wait for aborted builds forever #33

Merged
merged 4 commits into from
Nov 3, 2020
Merged

Don't wait for aborted builds forever #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86d07c9
Don't wait for aborted builds forever
Oct 30, 2020
62c1bc6
Address comment, fix unit test
Nov 2, 2020
6d1632f
Fix linting and coverage
Nov 3, 2020
13e8f5e
Forgot to remove console.log
Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const describeBuild = async ({ appSlug, buildSlug, client }) => {
const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => {
let lastActive = Date.now();
let timestamp;

let attributes;
do {
if (timestamp) {
await sleep(options.interval || 5000);
Expand Down Expand Up @@ -61,11 +61,19 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => {
lastActive = now;
}

// Sometimes build might have empty log_chunks, have is_archived set to false, have non-empty timestamp, but be aborted
// (because of bitrise timeout for example). Don't follow such builds forever:
attributes = await describeBuild({ appSlug, buildSlug, client });
if (attributes.status === 3 && !response.data.is_archived && response.data.log_chunks.length === 0) {
process.stdout.write('Build has been aborted, not polling logs any more\n');
throw new Error(`Build ${appSlug}/${buildSlug} aborted`);
}

timestamp = response.data.timestamp;
} while (timestamp);

const attributes = await describeBuild({ appSlug, buildSlug, client });

if (!attributes) {
attributes = await describeBuild({ appSlug, buildSlug, client });
}
if (attributes.status > 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Will this fail if the break on line 50 is hit before fetching the attributes? This file needs TypeScript for safety

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch! I've fixed this. Turns out one of the unit tests was failing because of this ("following a successful build that has already finished prints the log output"), but somehow I didn't notice this. Would you mind if I convert this to typescript in a follow-up PR?

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of a TypeScript conversion

throw new Error(`Build ${appSlug}/${buildSlug} failed`);
}
Expand Down
19 changes: 18 additions & 1 deletion test/build.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const test = require('ava');
const uuid = require('uuid/v4');

const { DateTime } = require('luxon');
const { stubAbortBuild, stubArchivedBuildLog, stubBuildLogStream, stubGetBuild, stubListArtifacts } = require('./stubs');
const { stubAbortBuild, stubArchivedBuildLog, stubBuildLogStream, stubGetBuild, stubListArtifacts, stubEmptyNonArchivedBuildLog } = require('./stubs');

test.beforeEach((test) => {
const appSlug = uuid();
Expand Down Expand Up @@ -105,6 +105,23 @@ test.serial('following a failed build that has not finished prints the log outpu
}
});

test.serial('following an aborted build with is_archived=false prints the log output and then errors', async (test) => {
const { appSlug, build, buildSlug, client } = test.context;
const buildStub = stubGetBuild({ appSlug, axios: client, buildSlug });
buildStub.build.status = 3;
stubEmptyNonArchivedBuildLog({ appSlug, axios: client, buildSlug });
// Cause timers to execute immediately
const clock = sinon.stub(global, 'setTimeout').callsArg(0);
const write = sinon.stub(process.stdout, 'write');

try {
await test.throwsAsync(() => build.follow(), { message: `Build ${buildStub.build.slug}/${buildStub.build.build_slug} aborted` });
} finally {
clock.restore();
write.restore();
}
});

test.serial('following a successful build that has already finished prints the log output', async (test) => {
const { appSlug, build, buildSlug, client } = test.context;
const logText = 'some log text';
Expand Down
14 changes: 14 additions & 0 deletions test/stubs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ exports.stubArchivedBuildLog = ({ appSlug, axios, buildSlug, logText }) => {
});
};

exports.stubEmptyNonArchivedBuildLog = ({ appSlug, axios, buildSlug }) => {
const logUrl = 'http://localhost/logs/example.txt';
const stub = getStub(axios, 'get');

stub.withArgs(`/apps/${appSlug}/builds/${buildSlug}/log`).resolves({
data: {
expiring_raw_log_url: logUrl,
is_archived: false,
log_chunks: []
},
status: 200
});
};

exports.stubBuildLogStream = ({ appSlug, axios, buildSlug, logChunks }) => {
const logStub = getStub(axios, 'get');

Expand Down
36 changes: 35 additions & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2094,11 +2094,18 @@ follow-redirects@1.5.10:
dependencies:
debug "=3.1.0"

for-in@^1.0.2:
for-in@^1.0.1, for-in@^1.0.2:
version "1.0.2"
resolved "https://registry.yarnpkg.com/for-in/-/for-in-1.0.2.tgz#81068d295a8142ec0ac726c6e2200c30fb6d5e80"
integrity sha1-gQaNKVqBQuwKxybG4iAMMPttXoA=

for-own@^0.1.4:
version "0.1.5"
resolved "https://registry.yarnpkg.com/for-own/-/for-own-0.1.5.tgz#5265c681a4f294dabbf17c9509b6763aa84510ce"
integrity sha1-UmXGgaTylNq78XyVCbZ2OqhFEM4=
dependencies:
for-in "^1.0.1"

foreground-child@^1.5.3, foreground-child@^1.5.6:
version "1.5.6"
resolved "https://registry.yarnpkg.com/foreground-child/-/foreground-child-1.5.6.tgz#4fd71ad2dfde96789b980a5c0a295937cb2f5ce9"
Expand Down Expand Up @@ -3131,6 +3138,14 @@ load-json-file@^5.2.0:
strip-bom "^3.0.0"
type-fest "^0.3.0"

locate-path@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-2.0.0.tgz#2b568b265eec944c6d9c0de9c3dbbbca0354cd8e"
integrity sha1-K1aLJl7slExtnA3pw9u7ygNUzY4=
dependencies:
p-locate "^2.0.0"
path-exists "^3.0.0"

locate-path@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-3.0.0.tgz#dbec3b3ab759758071b58fe59fc41871af21400e"
Expand Down Expand Up @@ -3768,13 +3783,27 @@ p-finally@^1.0.0:
resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae"
integrity sha1-P7z7FbiZpEEjs0ttzBi3JDNqLK4=

p-limit@^1.1.0:
version "1.3.0"
resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-1.3.0.tgz#b86bd5f0c25690911c7590fcbfc2010d54b3ccb8"
integrity sha512-vvcXsLAJ9Dr5rQOPk7toZQZJApBl2K4J6dANSsEuh6QI41JYcsS/qhTGa9ErIUUgK3WNQoJYvylxvjqmiqEA9Q==
dependencies:
p-try "^1.0.0"

p-limit@^2.0.0, p-limit@^2.2.0:
version "2.2.1"
resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-2.2.1.tgz#aa07a788cc3151c939b5131f63570f0dd2009537"
integrity sha512-85Tk+90UCVWvbDavCLKPOLC9vvY8OwEX/RtKF+/1OADJMVlFfEHOiMTPVyxg7mk/dKa+ipdHm0OUkTvCpMTuwg==
dependencies:
p-try "^2.0.0"

p-locate@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-2.0.0.tgz#20a0103b222a70c8fd39cc2e580680f3dde5ec43"
integrity sha1-IKAQOyIqcMj9OcwuWAaA893l7EM=
dependencies:
p-limit "^1.1.0"

p-locate@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-3.0.0.tgz#322d69a05c0264b25997d9f40cd8a891ab0064a4"
Expand All @@ -3794,6 +3823,11 @@ p-map@^2.0.0:
resolved "https://registry.yarnpkg.com/p-map/-/p-map-2.1.0.tgz#310928feef9c9ecc65b68b17693018a665cea175"
integrity sha512-y3b8Kpd8OAN444hxfBbFfj1FY/RjtTd8tzYwhUqNYXx0fXx2iX4maP4Qr6qhIKbQXI02wTLAda4fYUbDagTUFw==

p-try@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/p-try/-/p-try-1.0.0.tgz#cbc79cdbaf8fd4228e13f621f2b1a237c1b207b3"
integrity sha1-y8ec26+P1CKOE/Yh8rGiN8GyB7M=

p-try@^2.0.0:
version "2.2.0"
resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6"
Expand Down