Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore EEXIST error to fix an write after end issue #33

Merged
merged 1 commit into from
Jan 2, 2019
Merged

Ignore EEXIST error to fix an write after end issue #33

merged 1 commit into from
Jan 2, 2019

Conversation

vager
Copy link
Contributor

@vager vager commented Dec 29, 2018

No description provided.

@iccicci
Copy link
Owner

iccicci commented Dec 29, 2018

Hi @ccHare ,

first of all thank you for you interest in rotating-file-stream.

One question, can please you provide me an example failing without your patch?
the function makePath should be called only when the directory does not exists: I can't imagine a scenario when that function is called and directory exists.

Thank you,
iCC

@vager
Copy link
Contributor Author

vager commented Jan 2, 2019

Hi @iccicci

The following is a simple example I try to reproduce write after end error,

const fs = require('fs');
const rfs = require('rotating-file-stream');
const rimraf = require('rimraf');

let path = '/tmp/logdir';
rimraf.sync(path);

let stream1 = rfs('access.log', {
    path:     path,
    rotationTime: true
});
// Failed to create log file server.log in /tmp/logdir/
let stream2 = rfs('server.log', {
    path:     path,
    rotationTime: true
});

let rs = fs.createReadStream('/tmp/64M');
rs.pipe(stream2);
rs.pipe(stream1);

My failing code(encounter write after end) is more complicated than this simple example, but their logic is similar.

Thanks

@iccicci
Copy link
Owner

iccicci commented Jan 2, 2019

You are right!

Being all processes asynchronous, can exist (as you proved) cases where two concurrent directory creation attempts happen.

Thank you,
iCC

@iccicci iccicci merged commit f2175a7 into iccicci:master Jan 2, 2019
iccicci added a commit that referenced this pull request Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants