Skip to content

Commit

Permalink
Added error handling for zip files (#107)
Browse files Browse the repository at this point in the history
closes #106

GScan didn't handle errors `.zip` file errors fully. Furthermore, there is a bug in one of the underlying dependencies, which does not handle and return an error event correctly. Until max-mapper/extract-zip#65 is fixed, we therefore need to use the forked version, which includes the fix here: https://github.com/AileenCGN/extract-zip.git
  • Loading branch information
aileen committed May 15, 2018
1 parent 1fe0b73 commit 596ec75
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 23 deletions.
26 changes: 15 additions & 11 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,21 @@ app.post('/',
};
debug('Uploaded: ' + zip.name + ' to ' + zip.path);

gscan.checkZip(zip).then(function processResult(theme) {
debug('Checked: ' + zip.name);
res.theme = theme;
gscan.checkZip(zip)
.then(function processResult(theme) {
debug('Checked: ' + zip.name);
res.theme = theme;

debug('attempting to remove: ' + req.file.path);
pfs.removeDir(req.file.path)
.finally(function () {
debug('Calling next');
return next();
});
});
debug('attempting to remove: ' + req.file.path);
pfs.removeDir(req.file.path)
.finally(function () {
debug('Calling next');
return next();
});
}).catch(function (error) {
debug('Calling next with error');
return next(error);
});
},
function doRender(req, res) {
debug('Formatting result');
Expand All @@ -78,7 +82,7 @@ app.use(function (req, res, next) {
// eslint-disable-next-line no-unused-vars
app.use(function (err, req, res, next) {
req.err = err;
res.render('error', {message: err.message, stack: err.stack});
res.render('error', {message: err.message, stack: err.stack, details: err.errorDetails, context: err.context});
});

server.start(app);
3 changes: 3 additions & 0 deletions app/tpl/error.hbs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<h4 class="pricing-subheader center-text">Uh-Oh, There was an error.</h4>
<section class="center-text">
<p>{{message}}</p>
<p>{{context}}</p>
<p>{{details}}</p>
<p>{{stack}}</p>
<p>Please visit our <a href="https://forum.ghost.org">forum</a> and let us know!</p>
</section>
6 changes: 5 additions & 1 deletion bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ if (!program.args.length) {
} else {
if (program.zip) {
console.log('Checking zip file...');
gscan.checkZip(themePath).then(outputResults);
gscan.checkZip(themePath)
.then(outputResults)
.catch(function (error) {
console.error(error);
});
} else {
console.log('Checking directory...');
gscan.check(themePath).then(outputResults).catch(function ENOTDIRPredicate(err) {
Expand Down
8 changes: 8 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var _ = require('lodash'),
checker = require('./checker'),
format = require('./format'),
readZip = require('./read-zip'),
errors = require('ghost-ignition').errors,
checkZip;

checkZip = function checkZip(themePath, options) {
Expand Down Expand Up @@ -31,6 +32,13 @@ checkZip = function checkZip(themePath, options) {
return pfs.removeDir(zip.origPath).then(function returnResult() {
return theme;
});
}).catch(function (error) {
throw new errors.ValidationError({
message: 'Failed to read zip file',
help: 'Your zip file might be corrupted, try unzipping and zipping again.',
errorDetails: error.message,
context: zip.name
});
});
};

Expand Down
2 changes: 1 addition & 1 deletion lib/read-zip.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ var debug = require('ghost-ignition').debug('zip'),
Promise = require('bluebird'),
os = require('os'),
glob = require('glob'),
extract = require('extract-zip'),
extract = require('@tryghost/extract-zip'),
uuid = require('uuid'),
_ = require('lodash'),
readZip,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@
"gscan": "./bin/cli.js"
},
"dependencies": {
"@tryghost/extract-zip": "1.6.6",
"bluebird": "^3.4.6",
"chalk": "^1.1.1",
"commander": "2.15.1",
"express": "^4.16.2",
"express-hbs": "^1.0.3",
"extract-zip": "^1.6.5",
"fs-extra": "^0.26.2",
"ghost-ignition": "2.9.2",
"glob": "^7.0.5",
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@
lodash "^4.17.4"
shipit-cli "^3.0.0"

"@tryghost/extract-zip@1.6.6":
version "1.6.6"
resolved "https://registry.yarnpkg.com/@tryghost/extract-zip/-/extract-zip-1.6.6.tgz#937e0e775fec6dea937ac49d73a068bcafb67f50"
dependencies:
concat-stream "1.6.0"
debug "2.6.9"
mkdirp "0.5.0"
yauzl "2.4.1"

abbrev@1:
version "1.1.1"
resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8"
Expand Down Expand Up @@ -1201,15 +1210,6 @@ extglob@^2.0.4:
snapdragon "^0.8.1"
to-regex "^3.0.1"

extract-zip@^1.6.5:
version "1.6.6"
resolved "https://registry.yarnpkg.com/extract-zip/-/extract-zip-1.6.6.tgz#1290ede8d20d0872b429fd3f351ca128ec5ef85c"
dependencies:
concat-stream "1.6.0"
debug "2.6.9"
mkdirp "0.5.0"
yauzl "2.4.1"

extsprintf@1.3.0:
version "1.3.0"
resolved "https://registry.yarnpkg.com/extsprintf/-/extsprintf-1.3.0.tgz#96918440e3041a7a414f8c52e3c574eb3c3e1e05"
Expand Down

0 comments on commit 596ec75

Please sign in to comment.