Comments (8)
Thanks @contra. So this is tricky because autodrain returns a promise that could be of use (i.e. resolve when success or reject when error). So I'm not sure that the fix here is to remove the promise altogether.
If you either await
the autodrain (inside a function that is async) the warning should go away? Do you have any other suggestions?
from node-unzipper.
@ZJONSSON Yeah, adding .then(() => null).catch(() => null)
will hide the warning but ultimately doesn't solve the issue of a promise hanging around. Maybe have it match the other API and do this?
entry.autodrain = function() {
__autodraining = true;
return {
promise: function() {
return new Promise(function(resolve,reject) {
entry.pipe(NoopStream());
entry.on('finish',resolve);
entry.on('error',reject);
});
}
};
};
So if you did want to await it, it's await entry.autodrain().promise()
.
from node-unzipper.
You still would have to actually pipe the entry into a NoopStream()
as the underlying entry is a PullStream
, i.e. to get to next entry you need to pull
the entire body of the entry you want to autodrain (and just discard the data).
Also if you are not interested in the promise, handling any errors that might occur in autodraining can be problematic (the error event handler is not accessible).
One way to solve this would be to return a stream that passes no data, but includes the error event handlers, but also includes a .promise
method.
Something like this:
entry.autodrain = function() {
__autodraining = true;
var draining = entry.pipe(NoopStream();
draining.promise = function() {
draining.on('finish',resolve);
draining.on('error',reject);
};
return draining;
}
Here you could then do entry.autodrain().on('error', e => dosomething)
or entry.autodrain().promise().catch(e => dosomething)
The problem with above suggestions is that both bring breaking changes, which I try to avoid unless there is a larger change to the API.
Maybe the right solution would be to allow the first argument to be "noPromise". i.e.:
entry.autodrain = function(noPromise) {
__autodraining = true;
var draining = entry.pipe(NoopStream();
if (noPromise) return draining;
draining.promise = function() {
draining.on('finish',resolve);
draining.on('error',reject);
};
return draining;
}
No argument is currently defined on .autodrain
so this would not technically be a breaking change, the error is not automatically swallowed and you can bypass the empty promise if you want.
What do you think?
from node-unzipper.
Is it documented anywhere that this returns a promise? I’m open to either, i believe in pulling bandaids but up to you if you think it’s worthy of a major bump.
from node-unzipper.
My last suggestion would not require a bump, and would allow you the same interface as you suggested, except you would have to entry.autodrain(true).on('error', handler)
would be promiseless
from node-unzipper.
@ZJONSSON Yeah, that works
from node-unzipper.
Hey @contra I looked more closely at this and I agree with you, i.e. as the returned promise is neither in documents nor any of the tests and examples we can just make the switch.
Did a PR but some of test that rely on external urls are failing. Probably unrelated, will look into that separately.
from node-unzipper.
published as + [email protected]
from node-unzipper.
Related Issues (20)
- Attempt to decompress latest Maxmind GeoLite archive causes the error HOT 1
- `Open.file()` returns unicode replacement character(�) for unicode filenames HOT 1
- stream is not async iterable (DuplexWrapper)
- Corrupted unzipped files when using Extract HOT 15
- 支持哪几种格式的解压? HOT 1
- unzipper failed. Emitted 'error' event on DuplexWrapper instance at HOT 2
- Issue with Incorrect Character Display in `fileName` HOT 2
- Open s3 very slow HOT 1
- How to use Open.url with native "fetch"? HOT 8
- buffers has no license HOT 4
- Byte Range for File Within an Archive HOT 5
- uncompressedSize always return 0 if zip file is download from google drive HOT 1
- 是否可以在 arkts 中使用
- Is there a way to pause reading entries the unzipper stream? HOT 2
- TypeError: Promise.map is not a function after upgrade to 0.11.3 HOT 4
- v0.11.3 is broken ? HOT 7
- Consider bounding the stream request when streaming entries HOT 3
- Error handling with customSource not hooked up correcly? HOT 3
- unzipper 0.11.5 completely breaks reading of docx files HOT 7
- Access the raw compressed data of an entry HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from node-unzipper.