From b7d6d3d9a1e4a2f9d3691d9003c0362e313ffdc2 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Sun, 24 Jul 2016 15:32:00 -0400 Subject: [PATCH] feat(GUI): display a nice alert ribbon if drive runs out of space (#588) We try our best to check that the images the user select are too big for the selected drive as early as possible, but this probes to be problematic with certain compressed formats, like bzip2, which doesn't store any information about the uncompressed size, requiring a ~50s intensive computation as a minimum to find it out. For these kinds of formats, we don't perform an early check, but instead gracefully handle the case where the drive doesn't have any more space. This PR handles an `ENOSPC` error by displaying the alert orange ribbon, and prompting the user to retry with a larger drive. This is a huge improvement over the cryptic `EIO` error what was thrown before, and over having Etcher freeze at a certain percentage point. Change-Type: minor Changelog-Entry: Display a nice alert ribbon if drive runs out of space. See: https://github.com/resin-io/etcher/issues/571 Signed-off-by: Juan Cruz Viotti --- lib/cli/etcher.js | 5 +-- lib/cli/writer.js | 3 +- lib/gui/app.js | 8 +++++ lib/gui/models/store.js | 4 +++ lib/gui/modules/image-writer.js | 3 +- lib/gui/partials/main.html | 7 +++-- lib/src/child-writer/index.js | 10 ++++++ npm-shrinkwrap.json | 42 +++++++++++++++++++++++--- package.json | 2 +- tests/gui/modules/image-writer.spec.js | 22 +++++++++++++- 10 files changed, 92 insertions(+), 14 deletions(-) diff --git a/lib/cli/etcher.js b/lib/cli/etcher.js index edd0a2d2..15e28f88 100644 --- a/lib/cli/etcher.js +++ b/lib/cli/etcher.js @@ -65,7 +65,7 @@ form.run([ }); if (!selectedDrive) { - throw new Error(`Drive not found: ${selectedDrive}`); + throw new Error(`Drive not found: ${answers.drive}`); } return writer.writeImage(options._[0], selectedDrive, { @@ -120,7 +120,8 @@ form.run([ log.toStderr(JSON.stringify({ command: 'error', data: { - message: error.message + message: error.message, + code: error.code } })); } else { diff --git a/lib/cli/writer.js b/lib/cli/writer.js index 8bff86e1..74a44d82 100644 --- a/lib/cli/writer.js +++ b/lib/cli/writer.js @@ -58,9 +58,8 @@ exports.writeImage = (imagePath, drive, options, onProgress) => { return umount.umountAsync(drive.device).then(() => { return imageStream.getFromFilePath(imagePath); }).then((image) => { - return imageWrite.write(drive.device, image.stream, { + return imageWrite.write(drive, image, { check: options.validateWriteOnSuccess, - size: image.size, transform: image.transform }); }).then((writer) => { diff --git a/lib/gui/app.js b/lib/gui/app.js index c598b0ba..0e1bdd13 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -156,6 +156,14 @@ app.controller('AppController', function( this.tooltipModal = TooltipModalService; this.handleError = (error) => { + + // This particular error is handled by the alert ribbon + // on the main application page. + if (error.code === 'ENOSPC') { + AnalyticsService.logEvent('Drive ran out of space'); + return; + } + OSDialogService.showError(error); // Also throw it so it gets displayed in DevTools diff --git a/lib/gui/models/store.js b/lib/gui/models/store.js index 29bb3215..fd1923f1 100644 --- a/lib/gui/models/store.js +++ b/lib/gui/models/store.js @@ -206,6 +206,10 @@ const storeReducer = (state, action) => { throw new Error(`Invalid results sourceChecksum: ${action.data.sourceChecksum}`); } + if (action.data.errorCode && !_.isString(action.data.errorCode)) { + throw new Error(`Invalid results errorCode: ${action.data.errorCode}`); + } + return state .set('isFlashing', false) .set('flashResults', Immutable.fromJS(action.data)) diff --git a/lib/gui/modules/image-writer.js b/lib/gui/modules/image-writer.js index 3cb32816..c356a492 100644 --- a/lib/gui/modules/image-writer.js +++ b/lib/gui/modules/image-writer.js @@ -252,7 +252,8 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) }).catch((error) => { this.unsetFlashingFlag({ cancelled: false, - passedValidation: false + passedValidation: false, + errorCode: error.code }); return $q.reject(error); diff --git a/lib/gui/partials/main.html b/lib/gui/partials/main.html index bcca9434..19c02a33 100644 --- a/lib/gui/partials/main.html +++ b/lib/gui/partials/main.html @@ -98,10 +98,13 @@
- + + Not enough space on the drive.
Please insert larger one and + + Your removable drive did not pass validation check.
Please insert another one and - + Oops, seems something went wrong. Click to retry
diff --git a/lib/src/child-writer/index.js b/lib/src/child-writer/index.js index 06443ec9..b4ccff5c 100644 --- a/lib/src/child-writer/index.js +++ b/lib/src/child-writer/index.js @@ -86,6 +86,16 @@ exports.write = (image, drive, options) => { }); child.on('message', (message) => { + + // The error object is decomposed by the CLI for serialisation + // purposes. We compose it back to an `Error` here in order + // to provide better encapsulation. + if (message.command === 'error') { + const error = new Error(message.data.message); + error.code = message.data.code; + return emitter.emit('error', error); + } + emitter.emit(message.command, message.data); }); diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index bfcf3a72..f6ef6420 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -307,9 +307,9 @@ "resolved": "https://registry.npmjs.org/astw/-/astw-2.0.0.tgz" }, "async": { - "version": "2.0.0-rc.6", + "version": "2.0.0", "from": "async@>=2.0.0-rc.2 <3.0.0", - "resolved": "https://registry.npmjs.org/async/-/async-2.0.0-rc.6.tgz" + "resolved": "https://registry.npmjs.org/async/-/async-2.0.0.tgz" }, "async-foreach": { "version": "0.1.3", @@ -1278,9 +1278,31 @@ "resolved": "https://registry.npmjs.org/etcher-image-stream/-/etcher-image-stream-2.5.2.tgz" }, "etcher-image-write": { - "version": "5.0.3", - "from": "etcher-image-write@>=5.0.3 <6.0.0", - "resolved": "https://registry.npmjs.org/etcher-image-write/-/etcher-image-write-5.0.3.tgz" + "version": "6.0.0", + "from": "etcher-image-write@6.0.0", + "resolved": "https://registry.npmjs.org/etcher-image-write/-/etcher-image-write-6.0.0.tgz", + "dependencies": { + "isarray": { + "version": "1.0.0", + "from": "isarray@~1.0.0", + "resolved": "http://registry.npmjs.org/isarray/-/isarray-1.0.0.tgz" + }, + "readable-stream": { + "version": "2.0.6", + "from": "readable-stream@~2.0.0", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.0.6.tgz" + }, + "through2": { + "version": "2.0.1", + "from": "through2@>=2.0.1 <3.0.0", + "resolved": "https://registry.npmjs.org/through2/-/through2-2.0.1.tgz" + }, + "xtend": { + "version": "4.0.1", + "from": "xtend@~4.0.0", + "resolved": "https://registry.npmjs.org/xtend/-/xtend-4.0.1.tgz" + } + } }, "etcher-latest-version": { "version": "1.0.0", @@ -1615,6 +1637,11 @@ "from": "glob2base@>=0.0.12 <0.0.13", "resolved": "https://registry.npmjs.org/glob2base/-/glob2base-0.0.12.tgz" }, + "globals": { + "version": "9.9.0", + "from": "globals@>=9.2.0 <10.0.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-9.9.0.tgz" + }, "globby": { "version": "4.1.0", "from": "globby@>=4.0.0 <5.0.0", @@ -4364,6 +4391,11 @@ "from": "shell-quote@>=1.4.3 <2.0.0", "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.6.1.tgz" }, + "shelljs": { + "version": "0.6.0", + "from": "shelljs@>=0.6.0 <0.7.0", + "resolved": "https://registry.npmjs.org/shelljs/-/shelljs-0.6.0.tgz" + }, "sigmund": { "version": "1.0.1", "from": "sigmund@>=1.0.0 <1.1.0", diff --git a/package.json b/package.json index 53f9e78b..d2505922 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "drivelist": "^3.2.2", "electron-is-running-in-asar": "^1.0.0", "etcher-image-stream": "^2.5.2", - "etcher-image-write": "^5.0.3", + "etcher-image-write": "^6.0.0", "etcher-latest-version": "^1.0.0", "file-tail": "^0.3.0", "flexboxgrid": "^6.3.0", diff --git a/tests/gui/modules/image-writer.spec.js b/tests/gui/modules/image-writer.spec.js index 5c67a02e..3430b539 100644 --- a/tests/gui/modules/image-writer.spec.js +++ b/tests/gui/modules/image-writer.spec.js @@ -241,6 +241,17 @@ describe('Browser: ImageWriter', function() { }).to.throw('Missing results'); }); + it('should throw if errorCode is defined but it is not a number', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234', + errorCode: 123 + }); + }).to.throw('Invalid results errorCode: 123'); + }); + it('should throw if no passedValidation', function() { m.chai.expect(function() { ImageWriterService.unsetFlashingFlag({ @@ -445,7 +456,9 @@ describe('Browser: ImageWriter', function() { beforeEach(function() { this.performWriteStub = m.sinon.stub(ImageWriterService, 'performWrite'); - this.performWriteStub.returns($q.reject(new Error('write error'))); + this.error = new Error('write error'); + this.error.code = 'FOO'; + this.performWriteStub.returns($q.reject(this.error)); }); afterEach(function() { @@ -458,6 +471,13 @@ describe('Browser: ImageWriter', function() { m.chai.expect(ImageWriterService.isFlashing()).to.be.false; }); + it('should set the error code in the flash results', function() { + ImageWriterService.flash('foo.img', '/dev/disk2'); + $rootScope.$apply(); + const flashResults = ImageWriterService.getFlashResults(); + m.chai.expect(flashResults.errorCode).to.equal('FOO'); + }); + it('should be rejected with the error', function() { ImageWriterService.unsetFlashingFlag({ passedValidation: true,