Skip to content

Commit

Permalink
ci(aio): do not fail when re-deploying preview for the same PR/SHA
Browse files Browse the repository at this point in the history
Previously, when trying to upload the build artifacts for a PR/SHA that was
already successfully deployed (e.g. when re-running a Travis job), the preview
server would return a 403 and the build would fail.

Since we have other mechanisms to verify that the PR author is trusted and the
artifacts do indeed come from the specified PR and since the new artifacts
should be the same with the already deployed ones (same SHA), there is no reason
to fail the build. The preview server will reject the request with a special
HTTP status code (409 - Conflict), which the `deploy-preview` script will
recognize and exit with 0.
  • Loading branch information
gkalpak authored and petebacondarwin committed Apr 13, 2017
1 parent 9cb5964 commit d263595
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class BuildCreator extends EventEmitter {
all([this.exists(prDir), this.exists(shaDir)]).
then(([prDirExisted, shaDirExisted]) => {
if (shaDirExisted) {
throw new UploadError(403, `Request to overwrite existing directory: ${shaDir}`);
throw new UploadError(409, `Request to overwrite existing directory: ${shaDir}`);
}

dirToRemoveOnError = prDirExisted ? shaDir : prDir;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
h.createDummyArchive(pr9, sha9, archivePath);

uploadBuild(pr9, sha9, archivePath).
then(h.verifyResponse(403)).
then(h.verifyResponse(409)).
then(() => Promise.all([
getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('upload-server (on HTTP)', () => {
expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content');

h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
then(h.verifyResponse(403, /^Request to overwrite existing directory/)).
then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
then(() => expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content')).
then(done);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('BuildCreator', () => {
it('should throw if the build does already exist', done => {
bcExistsSpy.and.returnValue(true);
bc.create(pr, sha, archive).catch(err => {
expectToBeUploadError(err, 403, `Request to overwrite existing directory: ${shaDir}`);
expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`);
done();
});
});
Expand Down
5 changes: 3 additions & 2 deletions aio/scripts/deploy-preview.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ httpCode=$(
| sed 's/HTTP_CODE: //'
)

# Exit with an error if the request failed
if [ $httpCode -lt 200 ] || [ $httpCode -ge 400 ]; then
# Exit with an error if the request failed.
# (Ignore 409 failures, which mean trying to re-deploy for the same PR/SHA.)
if [ $httpCode -lt 200 ] || ([ $httpCode -ge 400 ] && [ $httpCode -ne 409 ]); then
exit 1
fi

Expand Down

0 comments on commit d263595

Please sign in to comment.