Skip to content

Commit ba52b7b

Browse files
fengmk2fengmk2
authored
fix: impl _final method instead hack pipe event (#114)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated `.gitignore` to exclude `pnpm-lock.yaml` from version control. * Updated GitHub Actions workflows with a new merge group trigger and revised release job configuration. * **Refactor** * Improved ZIP file uncompression with enhanced error handling and centralized completion callbacks. * Added debug logging to improve traceability during ZIP extraction. * **Style** * Suppressed console log outputs in multiple test suites for cleaner test execution. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 8ac1164 commit ba52b7b

16 files changed

+127
-101
lines changed

‎.github/workflows/nodejs.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ on:
77
pull_request:
88
branches: [ master ]
99

10+
merge_group:
11+
1012
jobs:
1113
Job:
1214
name: Node.js
1315
uses: node-modules/github-actions/.github/workflows/node-test.yml@master
1416
with:
1517
os: 'ubuntu-latest, macos-latest, windows-latest'
1618
version: '18, 20, 22, 24'
19+
secrets:
20+
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

‎.github/workflows/release.yml‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ on:
66

77
jobs:
88
release:
9-
name: Node.js
10-
uses: node-modules/github-actions/.github/workflows/node-release.yml@master
9+
name: NPM
10+
uses: node-modules/github-actions/.github/workflows/npm-release.yml@master
1111
secrets:
12-
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
1312
GIT_TOKEN: ${{ secrets.GIT_TOKEN }}

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ test/fixtures/chinese-path-test.zip
77
.DS_Store
88
yarn.lock
99
!test/fixtures/symlink/node_modules
10+
pnpm-lock.yaml

‎lib/zip/uncompress_stream.js‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
// https://github.com/thejoshwolfe/yauzl#no-streaming-unzip-api
44

5+
const debug = require('util').debuglog('compressing/zip/uncompress_stream');
56
const yauzl = require('@eggjs/yauzl');
67
const stream = require('stream');
78
const UncompressBaseStream = require('../base_write_stream');
@@ -38,12 +39,20 @@ class ZipUncompressStream extends UncompressBaseStream {
3839
if (this._zipFileNameEncoding === 'utf-8') {
3940
this._zipFileNameEncoding = 'utf8';
4041
}
42+
this._finalCallback = err => {
43+
if (err) {
44+
debug('finalCallback, error: %j', err);
45+
return this.emit('error', err);
46+
}
47+
this.emit('finish');
48+
};
4149

4250
this[YAUZL_CALLBACK] = this[YAUZL_CALLBACK].bind(this);
4351

4452
const sourceType = utils.sourceType(opts.source);
4553

4654
const yauzlOpts = this._yauzlOpts = Object.assign({}, DEFAULTS, opts.yauzl);
55+
debug('sourceType: %s, yauzlOpts: %j', sourceType, yauzlOpts);
4756
if (sourceType === 'file') {
4857
yauzl.open(opts.source, yauzlOpts, this[YAUZL_CALLBACK]);
4958
return;
@@ -60,27 +69,26 @@ class ZipUncompressStream extends UncompressBaseStream {
6069
.catch(e => this.emit('error', e));
6170
return;
6271
}
63-
64-
this.on('pipe', srcStream => {
65-
srcStream.unpipe(srcStream);
66-
67-
utils.streamToBuffer(srcStream)
68-
.then(buf => {
69-
this._chunks.push(buf);
70-
buf = Buffer.concat(this._chunks);
71-
yauzl.fromBuffer(buf, yauzlOpts, this[YAUZL_CALLBACK]);
72-
})
73-
.catch(e => this.emit('error', e));
74-
});
7572
}
7673

77-
_write(chunk) {
78-
// push to _chunks array, this will only happen once, for stream will be unpiped.
74+
_write(chunk, _encoding, callback) {
7975
this._chunks.push(chunk);
76+
debug('write size: %d, chunks: %d', chunk.length, this._chunks.length);
77+
callback();
78+
}
79+
80+
_final(callback) {
81+
const buf = Buffer.concat(this._chunks);
82+
debug('final, buf size: %d, chunks: %d', buf.length, this._chunks.length);
83+
this._finalCallback = callback;
84+
yauzl.fromBuffer(buf, this._yauzlOpts, this[YAUZL_CALLBACK]);
8085
}
8186

8287
[YAUZL_CALLBACK](err, zipFile) {
83-
if (err) return this.emit('error', err);
88+
if (err) {
89+
debug('yauzl error', err);
90+
return this._finalCallback(err);
91+
}
8492

8593
zipFile.readEntry();
8694

@@ -106,17 +114,22 @@ class ZipUncompressStream extends UncompressBaseStream {
106114

107115
if (type === 'file') {
108116
zipFile.openReadStream(entry, (err, readStream) => {
109-
if (err) return this.emit('error', err);
117+
if (err) {
118+
debug('file, error: %j', err);
119+
return this._finalCallback(err);
120+
}
121+
debug('file, header: %j', header);
110122
this.emit('entry', header, readStream, next);
111123
});
112124
} else { // directory
113125
const placeholder = new stream.Readable({ read() {} });
126+
debug('directory, header: %j', header);
114127
this.emit('entry', header, placeholder, next);
115128
setImmediate(() => placeholder.emit('end'));
116129
}
117130
})
118-
.on('end', () => this.emit('finish'))
119-
.on('error', err => this.emit('error', err));
131+
.on('end', () => this._finalCallback())
132+
.on('error', err => this._finalCallback(err));
120133

121134
function next() {
122135
zipFile.readEntry();

‎test/gzip/file_stream.test.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe('test/gzip/file_stream.test.js', () => {
1111
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
1212
const sourceStream = fs.createReadStream(sourceFile);
1313
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
14-
console.log('destFile', destFile);
14+
// console.log('destFile', destFile);
1515
const gzipStream = new compressing.gzip.FileStream();
1616
const destStream = fs.createWriteStream(destFile);
1717
pump(sourceStream, gzipStream, destStream, err => {
@@ -24,7 +24,7 @@ describe('test/gzip/file_stream.test.js', () => {
2424
it('should compress according to file path', done => {
2525
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
2626
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
27-
console.log('destFile', destFile);
27+
// console.log('destFile', destFile);
2828
const gzipStream = new compressing.gzip.FileStream({ source: sourceFile });
2929
const destStream = fs.createWriteStream(destFile);
3030
pump(gzipStream, destStream, err => {
@@ -44,14 +44,14 @@ describe('test/gzip/file_stream.test.js', () => {
4444

4545
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
4646
await fs.promises.writeFile(destFile, Buffer.concat(gzipChunks));
47-
console.log(destFile);
47+
// console.log(destFile);
4848
});
4949

5050
it('should compress buffer', done => {
5151
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
5252
const sourceBuffer = fs.readFileSync(sourceFile);
5353
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
54-
console.log('destFile', destFile);
54+
// console.log('destFile', destFile);
5555
const destStream = fs.createWriteStream(destFile);
5656
const gzipStream = new compressing.gzip.FileStream({ source: sourceBuffer });
5757
pump(gzipStream, destStream, err => {
@@ -66,7 +66,7 @@ describe('test/gzip/file_stream.test.js', () => {
6666
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
6767
const sourceStream = fs.createReadStream(sourceFile);
6868
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
69-
console.log('destFile', destFile);
69+
// console.log('destFile', destFile);
7070
const destStream = fs.createWriteStream(destFile);
7171
const gzipStream = new compressing.gzip.FileStream({ source: sourceStream });
7272
pump(gzipStream, destStream, err => {

‎test/gzip/index.test.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ describe('test/gzip/index.test.js', () => {
1313
it('gzip.compressFile(file, stream)', async () => {
1414
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
1515
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
16-
console.log('destFile', destFile);
16+
// console.log('destFile', destFile);
1717
const fileStream = fs.createWriteStream(destFile);
1818
await compressing.gzip.compressFile(sourceFile, fileStream);
1919
assert(fs.existsSync(destFile));
@@ -38,7 +38,7 @@ describe('test/gzip/index.test.js', () => {
3838
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
3939
const sourceBuffer = fs.readFileSync(sourceFile);
4040
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
41-
console.log('destFile', destFile);
41+
// console.log('destFile', destFile);
4242
const fileStream = fs.createWriteStream(destFile);
4343
await compressing.gzip.compressFile(sourceBuffer, fileStream);
4444
assert(fs.existsSync(destFile));
@@ -48,7 +48,7 @@ describe('test/gzip/index.test.js', () => {
4848
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
4949
const sourceStream = fs.createReadStream(sourceFile);
5050
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.gz');
51-
console.log('destFile', destFile);
51+
// console.log('destFile', destFile);
5252
const fileStream = fs.createWriteStream(destFile);
5353
await compressing.gzip.compressFile(sourceStream, fileStream);
5454
assert(fs.existsSync(destFile));

‎test/tar/file_stream.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ describe('test/tar/file_stream.test.js', () => {
1515
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
1616
const sourceStream = fs.createReadStream(sourceFile);
1717
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
18-
console.log('dest', destFile);
18+
// console.log('dest', destFile);
1919

2020
mm(console, 'warn', msg => {
2121
assert(msg === 'You should specify the size of streamming data by opts.size to prevent all streaming data from loading into memory. If you are sure about memory cost, pass opts.suppressSizeWarning: true to suppress this warning');
@@ -34,7 +34,7 @@ describe('test/tar/file_stream.test.js', () => {
3434
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
3535
const sourceStream = fs.createReadStream(sourceFile);
3636
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
37-
console.log('dest', destFile);
37+
// console.log('dest', destFile);
3838

3939
mm(console, 'warn', msg => {
4040
assert(!msg);

‎test/tar/index.test.js‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ describe('test/tar/index.test.js', () => {
1616
it('tar.compressFile(file, stream)', async () => {
1717
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
1818
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
19-
console.log('dest', destFile);
19+
// console.log('dest', destFile);
2020
const fileStream = fs.createWriteStream(destFile);
2121
await compressing.tar.compressFile(sourceFile, fileStream);
2222
assert(fs.existsSync(destFile));
@@ -25,7 +25,7 @@ describe('test/tar/index.test.js', () => {
2525
it('tar.compressFile(file, stream, { relativePath })', async () => {
2626
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
2727
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
28-
console.log('dest', destFile);
28+
// console.log('dest', destFile);
2929
const fileStream = fs.createWriteStream(destFile);
3030
await compressing.tar.compressFile(sourceFile, fileStream, { relativePath: 'dd/dd.log' });
3131
assert(fs.existsSync(destFile));
@@ -35,7 +35,7 @@ describe('test/tar/index.test.js', () => {
3535
it('tar.compressFile(file, stream) should error if file not exist', async () => {
3636
const sourceFile = path.join(__dirname, '..', 'fixtures', 'not-exist.log');
3737
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
38-
console.log('dest', destFile);
38+
// console.log('dest', destFile);
3939
const fileStream = fs.createWriteStream(destFile);
4040
let err;
4141
try {
@@ -65,7 +65,7 @@ describe('test/tar/index.test.js', () => {
6565
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
6666
const sourceStream = fs.createReadStream(sourceFile);
6767
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
68-
console.log('dest', destFile);
68+
// console.log('dest', destFile);
6969
const fileStream = fs.createWriteStream(destFile);
7070
mm(console, 'warn', msg => {
7171
assert(msg === 'You should specify the size of streamming data by opts.size to prevent all streaming data from loading into memory. If you are sure about memory cost, pass opts.suppressSizeWarning: true to suppress this warning');
@@ -78,7 +78,7 @@ describe('test/tar/index.test.js', () => {
7878
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
7979
const sourceStream = fs.createReadStream(sourceFile);
8080
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
81-
console.log('destFile', destFile);
81+
// console.log('destFile', destFile);
8282
const fileStream = fs.createWriteStream(destFile);
8383
mm(console, 'warn', msg => {
8484
assert(!msg);
@@ -91,7 +91,7 @@ describe('test/tar/index.test.js', () => {
9191
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
9292
const sourceBuffer = fs.readFileSync(sourceFile);
9393
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
94-
console.log('dest', destFile);
94+
// console.log('dest', destFile);
9595
const fileStream = fs.createWriteStream(destFile);
9696
await compressing.tar.compressFile(sourceBuffer, fileStream, { relativePath: 'xx.log' });
9797
assert(fs.existsSync(destFile));
@@ -101,7 +101,7 @@ describe('test/tar/index.test.js', () => {
101101
const sourceFile = path.join(__dirname, '..', 'fixtures/xxx/bin');
102102
const originStat = fs.statSync(sourceFile);
103103
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
104-
console.log('dest', destFile);
104+
// console.log('dest', destFile);
105105
const fileStream = fs.createWriteStream(destFile);
106106
await compressing.tar.compressFile(sourceFile, fileStream);
107107
assert(fs.existsSync(destFile));
@@ -111,7 +111,7 @@ describe('test/tar/index.test.js', () => {
111111
await compressing.tar.uncompress(destFile, destDir);
112112
const stat = fs.statSync(path.join(destDir, 'bin'));
113113
assert(stat.mode === originStat.mode);
114-
console.log(destDir);
114+
// console.log(destDir);
115115
});
116116

117117
});
@@ -120,7 +120,7 @@ describe('test/tar/index.test.js', () => {
120120
it('tar.compressDir(dir, destFile)', async () => {
121121
const sourceDir = path.join(__dirname, '..', 'fixtures');
122122
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
123-
console.log('dest', destFile);
123+
// console.log('dest', destFile);
124124
await compressing.tar.compressDir(sourceDir, destFile);
125125
assert(fs.existsSync(destFile));
126126
});
@@ -129,7 +129,7 @@ describe('test/tar/index.test.js', () => {
129129
const sourceDir = path.join(__dirname, '..', 'fixtures');
130130
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
131131
const destStream = fs.createWriteStream(destFile);
132-
console.log('dest', destFile);
132+
// console.log('dest', destFile);
133133
await compressing.tar.compressDir(sourceDir, destStream);
134134
assert(fs.existsSync(destFile));
135135
});
@@ -138,15 +138,15 @@ describe('test/tar/index.test.js', () => {
138138
const sourceDir = path.join(__dirname, '..', 'fixtures');
139139
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
140140
const destStream = fs.createWriteStream(destFile);
141-
console.log('dest', destFile);
141+
// console.log('dest', destFile);
142142
await compressing.tar.compressDir(sourceDir, destStream, { ignoreBase: true });
143143
assert(fs.existsSync(destFile));
144144
});
145145

146146
it('tar.compressDir(dir, destStream) should return promise', async () => {
147147
const sourceDir = path.join(__dirname, '..', 'fixtures');
148148
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
149-
console.log('dest', destFile);
149+
// console.log('dest', destFile);
150150
await compressing.tar.compressDir(sourceDir, destFile);
151151
assert(fs.existsSync(destFile));
152152
});
@@ -155,7 +155,7 @@ describe('test/tar/index.test.js', () => {
155155
const sourceDir = path.join(__dirname, '..', 'fixtures');
156156
const destFile = path.join(os.tmpdir(), uuid.v4() + '.tar');
157157
const destStream = fs.createWriteStream(destFile);
158-
console.log('dest', destFile);
158+
// console.log('dest', destFile);
159159
setImmediate(() => {
160160
destStream.emit('error', new Error('xxx'));
161161
});

0 commit comments

Comments
 (0)