Skip to content

Commit ce5d52c

Browse files
author
Gareth Jones
committed
fix: fallback to copy+truncate on error
1 parent 83e2eb2 commit ce5d52c

File tree

3 files changed

+177
-50
lines changed

3 files changed

+177
-50
lines changed

‎lib/RollingFileWriteStream.js‎

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
const debug = require("debug")("streamroller:RollingFileWriteStream");
22
const fs = require("fs-extra");
3-
const zlib = require("zlib");
43
const path = require("path");
54
const newNow = require("./now");
65
const format = require("date-format");
76
const { Writable } = require("stream");
87
const fileNameFormatter = require("./fileNameFormatter");
98
const fileNameParser = require("./fileNameParser");
9+
const moveAndMaybeCompressFile = require("./moveAndMaybeCompressFile");
1010

1111
/**
1212
* RollingFileWriteStream is mainly used when writing to a file rolling by date or size.
@@ -257,55 +257,6 @@ class RollingFileWriteStream extends Writable {
257257
}
258258
}
259259

260-
const moveAndMaybeCompressFile = async (
261-
sourceFilePath,
262-
targetFilePath,
263-
needCompress
264-
) => {
265-
if (sourceFilePath === targetFilePath) {
266-
debug(
267-
`moveAndMaybeCompressFile: source and target are the same, not doing anything`
268-
);
269-
return;
270-
}
271-
try {
272-
await fs.access(sourceFilePath, fs.constants.W_OK | fs.constants.R_OK);
273-
274-
debug(
275-
`moveAndMaybeCompressFile: moving file from ${sourceFilePath} to ${targetFilePath} ${
276-
needCompress ? "with" : "without"
277-
} compress`
278-
);
279-
if (needCompress) {
280-
await new Promise((resolve, reject) => {
281-
fs.createReadStream(sourceFilePath)
282-
.pipe(zlib.createGzip())
283-
.pipe(fs.createWriteStream(targetFilePath))
284-
.on("finish", () => {
285-
debug(
286-
`moveAndMaybeCompressFile: finished compressing ${targetFilePath}, deleting ${sourceFilePath}`
287-
);
288-
fs.unlink(sourceFilePath)
289-
.then(resolve)
290-
.catch(reject);
291-
});
292-
});
293-
} else {
294-
debug(
295-
`moveAndMaybeCompressFile: deleting file=${targetFilePath}, renaming ${sourceFilePath} to ${targetFilePath}`
296-
);
297-
await fs.unlink(targetFilePath).catch(() => {
298-
/* doesn't matter */
299-
});
300-
await fs.rename(sourceFilePath, targetFilePath);
301-
}
302-
} catch (e) {
303-
debug(
304-
`moveAndMaybeCompressFile: source file path does not exist. not moving. sourceFilePath=${sourceFilePath}`
305-
);
306-
}
307-
};
308-
309260
const deleteFiles = fileNames => {
310261
debug(`deleteFiles: files to delete: ${fileNames}`);
311262
return Promise.all(fileNames.map(f => fs.unlink(f)));

‎lib/moveAndMaybeCompressFile.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
const debug = require('debug')('streamroller:moveAndMaybeCompressFile');
2+
const fs = require('fs-extra');
3+
const zlib = require('zlib');
4+
5+
const moveAndMaybeCompressFile = async (
6+
sourceFilePath,
7+
targetFilePath,
8+
needCompress
9+
) => {
10+
if (sourceFilePath === targetFilePath) {
11+
debug(
12+
`moveAndMaybeCompressFile: source and target are the same, not doing anything`
13+
);
14+
return;
15+
}
16+
if (await fs.pathExists(sourceFilePath)) {
17+
18+
debug(
19+
`moveAndMaybeCompressFile: moving file from ${sourceFilePath} to ${targetFilePath} ${
20+
needCompress ? "with" : "without"
21+
} compress`
22+
);
23+
if (needCompress) {
24+
await new Promise((resolve, reject) => {
25+
fs.createReadStream(sourceFilePath)
26+
.pipe(zlib.createGzip())
27+
.pipe(fs.createWriteStream(targetFilePath))
28+
.on("finish", () => {
29+
debug(
30+
`moveAndMaybeCompressFile: finished compressing ${targetFilePath}, deleting ${sourceFilePath}`
31+
);
32+
fs.unlink(sourceFilePath)
33+
.then(resolve)
34+
.catch(() => {
35+
debug(`Deleting ${sourceFilePath} failed, truncating instead`);
36+
fs.truncate(sourceFilePath).then(resolve).catch(reject)
37+
});
38+
});
39+
});
40+
} else {
41+
debug(
42+
`moveAndMaybeCompressFile: deleting file=${targetFilePath}, renaming ${sourceFilePath} to ${targetFilePath}`
43+
);
44+
try {
45+
await fs.move(sourceFilePath, targetFilePath, { overwrite: true });
46+
} catch (e) {
47+
debug(
48+
`moveAndMaybeCompressFile: error moving ${sourceFilePath} to ${targetFilePath}`, e
49+
);
50+
debug(`Trying copy+truncate instead`);
51+
await fs.copy(sourceFilePath, targetFilePath, { overwrite: true });
52+
await fs.truncate(sourceFilePath);
53+
}
54+
}
55+
}
56+
};
57+
58+
module.exports = moveAndMaybeCompressFile;

‎test/moveAndMaybeCompressFile-test.js‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
require("should");
2+
3+
const fs = require('fs-extra');
4+
const path = require('path');
5+
const zlib = require('zlib');
6+
const proxyquire = require('proxyquire').noPreserveCache();
7+
const moveAndMaybeCompressFile = require('../lib/moveAndMaybeCompressFile');
8+
const TEST_DIR = path.normalize(`/tmp/moveAndMaybeCompressFile_${Math.floor(Math.random()*10000)}`);
9+
10+
describe('moveAndMaybeCompressFile', () => {
11+
beforeEach(async () => {
12+
await fs.emptyDir(TEST_DIR);
13+
});
14+
15+
after(async () => {
16+
await fs.remove(TEST_DIR);
17+
});
18+
19+
it('should move the source file to a new destination', async () => {
20+
const source = path.join(TEST_DIR, 'test.log');
21+
const destination = path.join(TEST_DIR, 'moved-test.log');
22+
await fs.outputFile(source, 'This is the test file.');
23+
await moveAndMaybeCompressFile(source, destination);
24+
25+
const contents = await fs.readFile(destination, 'utf8');
26+
contents.should.equal('This is the test file.');
27+
28+
const exists = await fs.pathExists(source);
29+
exists.should.be.false();
30+
31+
});
32+
33+
it('should compress the source file at the new destination', async () => {
34+
const source = path.join(TEST_DIR, 'test.log');
35+
const destination = path.join(TEST_DIR, 'moved-test.log.gz');
36+
await fs.outputFile(source, 'This is the test file.');
37+
await moveAndMaybeCompressFile(source, destination, true);
38+
39+
const zippedContents = await fs.readFile(destination);
40+
const contents = await new Promise(resolve => {
41+
zlib.gunzip(zippedContents, (e, data) => {
42+
resolve(data.toString());
43+
});
44+
});
45+
contents.should.equal('This is the test file.');
46+
47+
const exists = await fs.pathExists(source);
48+
exists.should.be.false();
49+
});
50+
51+
it('should do nothing if the source file and destination are the same', async () => {
52+
const source = path.join(TEST_DIR, 'pants.log');
53+
const destination = path.join(TEST_DIR, 'pants.log');
54+
await fs.outputFile(source, 'This is the test file.');
55+
await moveAndMaybeCompressFile(source, destination);
56+
57+
(await fs.readFile(source, 'utf8')).should.equal('This is the test file.');
58+
});
59+
60+
it('should do nothing if the source file does not exist', async () => {
61+
const source = path.join(TEST_DIR, 'pants.log');
62+
const destination = path.join(TEST_DIR, 'moved-pants.log');
63+
await moveAndMaybeCompressFile(source, destination);
64+
65+
(await fs.pathExists(destination)).should.be.false();
66+
});
67+
68+
it('should use copy+truncate if source file is locked (windows)', async () => {
69+
const moveWithMock = proxyquire('../lib/moveAndMaybeCompressFile', {
70+
"fs-extra": {
71+
exists: () => Promise.resolve(true),
72+
move: () => Promise.reject({ code: 'EBUSY', message: 'all gone wrong'}),
73+
copy: (fs.copy.bind(fs)),
74+
truncate: (fs.truncate.bind(fs))
75+
}
76+
});
77+
78+
const source = path.join(TEST_DIR, 'test.log');
79+
const destination = path.join(TEST_DIR, 'moved-test.log');
80+
await fs.outputFile(source, 'This is the test file.');
81+
await moveWithMock(source, destination);
82+
83+
const contents = await fs.readFile(destination, 'utf8');
84+
contents.should.equal('This is the test file.');
85+
86+
// won't delete the source, but it will be empty
87+
(await fs.readFile(source, 'utf8')).should.be.empty()
88+
89+
});
90+
91+
it('should truncate file if remove fails when compressed (windows)', async () => {
92+
const moveWithMock = proxyquire('../lib/moveAndMaybeCompressFile', {
93+
"fs-extra": {
94+
exists: () => Promise.resolve(true),
95+
unlink: () => Promise.reject({ code: 'EBUSY', message: 'all gone wrong'}),
96+
createReadStream: fs.createReadStream.bind(fs),
97+
truncate: fs.truncate.bind(fs)
98+
}
99+
});
100+
101+
const source = path.join(TEST_DIR, 'test.log');
102+
const destination = path.join(TEST_DIR, 'moved-test.log.gz');
103+
await fs.outputFile(source, 'This is the test file.');
104+
await moveWithMock(source, destination, true);
105+
106+
const zippedContents = await fs.readFile(destination);
107+
const contents = await new Promise(resolve => {
108+
zlib.gunzip(zippedContents, (e, data) => {
109+
resolve(data.toString());
110+
});
111+
});
112+
contents.should.equal('This is the test file.');
113+
114+
// won't delete the source, but it will be empty
115+
(await fs.readFile(source, 'utf8')).should.be.empty()
116+
117+
});
118+
});

0 commit comments

Comments
 (0)