Skip to content

Commit

Permalink
Account for gifsicle outputting warnings on stderr.
Browse files Browse the repository at this point in the history
Previously any output on stderr would be treated unconditionally as
signifying an error occurred. Turns out gifsicle may write warnings
to stderr - add logic to detect these and treat them as non-errors.
  • Loading branch information
alexjeffburke committed Dec 18, 2023
1 parent 8c70047 commit e833960
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 8 deletions.
40 changes: 33 additions & 7 deletions src/StdinoutStream.js
Expand Up @@ -3,20 +3,32 @@ const Stream = require('stream');

const debug = require('./debug');

function noopFilterStderr() {
return true;
}

module.exports = class StdinoutStream extends Stream.Transform {
constructor(name, binary, args) {
constructor(name, binary, args, options) {
super();

this.args = args;
this.binary = binary;
this.errorline = null;
this.filterStderr = null;
this.logger = null;
this.process = null;
this.hasEnded = false;
this.hasErrored = false;
this.hasOutput = false;

this.logger = debug(name);

options = options || {};
if (typeof options.filterStderr === 'function') {
this.filterStderr = options.filterStderr;
} else {
this.filterStderr = noopFilterStderr;
}
}

// implementation local methods
Expand Down Expand Up @@ -81,14 +93,28 @@ module.exports = class StdinoutStream extends Stream.Transform {
const stdinError = boundError('STDIN');
const stdoutError = boundError('STDOUT');

const decodeStderr = (line) => {
let errorMessage;
try {
errorMessage = line.toString();
} catch (e) {
return 'undecodeable output on stderr';
}
if (!errorMessage) {
// empty line output on stderr => ignore
return null;
} else if (!this.filterStderr(errorMessage)) {
// filter indicated a non-error => ignore
return null;
}
return errorMessage;
};

this.process.stderr.on('data', (line) => {
if (this.errorline === null) {
if (this.errorline !== null) {
// ignore
} else if ((this.errorline = decodeStderr(line)) !== null) {
this.hasErrored = true;
try {
this.errorline = line.toString();
} catch (e) {
this.errorline = 'undecodeable error';
}
}
});

Expand Down
6 changes: 5 additions & 1 deletion src/engines/gifsicle.js
Expand Up @@ -76,7 +76,11 @@ module.exports = {

function flush() {
if (args.length > 0) {
pipeline._attach(new StdinoutStream('gifsicle', binPath, args));
pipeline._attach(
new StdinoutStream('gifsicle', binPath, args, {
filterStderr: (line) => !line.startsWith('gifsicle: warning:'),
})
);
seenOperationThatMustComeBeforeExtract = false;
if (allGifsicleArgs.length > 0) allGifsicleArgs.push(';');
allGifsicleArgs.push(...args);
Expand Down
10 changes: 10 additions & 0 deletions test/impro.spec.js
Expand Up @@ -1112,6 +1112,16 @@ describe('impro', () => {
load('bulbInterlaced.gif')
));

it('should process any image that generates warnings to stderr', () =>
expect(
'lasercat.gif',
'when piped through',
impro.gifsicle().progressive(),
'to yield output satisfying',
'to have mime type',
'image/gif'
));

it('should support resize (only width)', () => {
const executeSpy = sinon.spy(impro.engineByName.gifsicle, 'execute');

Expand Down
Binary file added testdata/lasercat.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit e833960

Please sign in to comment.