Unhandled promise rejections #189
Comments
|
I will definitely get these fixed, but have you seen them in node? I’ve only seen these errors in the browser, which doesn’t seem as effective at detecting that promises are actually handled in a promise above them. |
|
I haven't reproduced them in node yet. I'll try to do it later today/tomorrow. Simulating network latency or instability should work. But except for the last to links, these promises are not returned nor assigned, so there's no promise above/bellow them in the promise chain. For example, https://github.com/ethers-io/ethers.js/blob/master/providers/json-rpc-provider.js#L124 is |
|
Getting back to that example, if you want the promise returned by function check() {
provider.getTransaction(hash).then(function(transaction) {
if (!transaction) {
setTimeout(check, 1000);
return;
}
transaction.wait = function() {
return provider.waitForTransaction(hash);
};
resolve(transaction);
})
.catch(reject); // <------ This line is new
}If you can clarify if this is the intended behavior, I'm happy to submit a PR fixing all the cases. |
|
Here's a reproduction script that works in node. It uses toxy and cors to create a proxy that simulates an bad connection, making 25% of the HTTP request fail. It sends a transaction every second. Some of them will fail gracefully, but eventually one will result in an unhandled promise rejection (see: https://github.com/ethers-io/ethers.js/blob/master/providers/json-rpc-provider.js#L124) and the script will exit. const ethers = require("ethers");
const toxy = require("toxy");
const cors = require("cors");
const JSONRPC_PROVIDER_URL = "http://localhost:8545";
const PROXY_PORT = 3000;
const CONNECTION_FAILURE_PORBABILITY = 25;
function createFaultyProxy() {
const proxy = toxy({
forwardHost: true,
timeout: 6e4,
proxyTimeout: 6e4
});
proxy.use(cors());
proxy.forward(JSONRPC_PROVIDER_URL);
proxy
.all("/*")
.poison(toxy.poisons.abort())
.rule(toxy.rules.probability(CONNECTION_FAILURE_PORBABILITY));
proxy.listen(PROXY_PORT);
console.log(
`Proxy listening on port ${PROXY_PORT}, redirecting everything to ${JSONRPC_PROVIDER_URL} with ${CONNECTION_FAILURE_PORBABILITY}% of chances of failing.`
);
}
function sendTransaction(provider) {
provider
.listAccounts()
.then(addresses => provider.getSigner(addresses[0]))
.then(signer => signer.sendTransaction({ to: signer.address, value: 123 }))
.catch(() =>
console.log("Error from outer promise, not from check(): ignored")
);
}
createFaultyProxy();
const provider = new ethers.providers.JsonRpcProvider(
`http://localhost:${PROXY_PORT}`
);
process.on("unhandledRejection", (error, promise) => {
console.error("Unhandled Rejection ", error);
process.exit(1);
});
setInterval(() => sendTransaction(provider), 1000); |
|
This is also happening to me in Node! Here is my code! It works perfectly for 3-4 minutes and then I get this error: ` ` const provider = new ethers.providers.JsonRpcProvider( const Web3 = require('web3') const web3 = new Web3(new Web3.providers.HttpProvider( provider.on( ) } catch (err) { |
|
I will be getting to this today. :) |
|
Thanks! I am using ethers.js for a project at work so having this up and running will be very helpful to me :) |
|
@ricmoo any updates on this, have some projects dependent on this library. |
|
Does the library fail to work in these cases? Or is it just noisy? I've been focussing all my time on getting the TypeScript library up to date and will have this fixed in that branch soon. But if this is actually causing problems (other than unwanted logs) I can back port the fix to this branch too. I'm hoping to get the new version out this week, with lots of new bells and whistles, and better APIs for a few features. :) |
|
I believe it may be the root cause of memory leaks in my project, which is a long-running monitoring tool, the memory leaks appear to occur when these unhandled rejections are logged. EDIT: Speaking of which, here is a recent log of said application after running for a few days: Basically has an endless amount of lines of these errors and then finally |
|
Oh, I see. I can definitely see how that could leak memory for a system tracking the promises. Sorry for the delay, I will fix this in both the 3.0 and 4.0 branch tomorrow. |
|
@alcuadrado Huge thanks! That scripted helped reproduce the error immediately. I've also added the eslint to help isolate the issues. In the future, it will be part of the standard build process, but some of the rules need to be tweaked. I will add the changes to the v4 branch later, since I think some refactoring I plan will help reduce the number of places I need to worry about. Once Travis CI is complete, I'll publish to npm. |
|
I need to increase the timeout on some test cases. I think I’m being throttled more by Travis CI since I’ve added more target platforms to test. I’ll get to that shortly. |
|
This has been published to npm as 3.0.25. I'm going to leave the issue open until all the updates to the 4.0 branch have also been updated, but for the 3.0 users, it should be good to go. :) |
|
I'm glad that the script helped you @ricmoo ! Thanks for fixing it :) |
|
This has been fixed in the v4 (TypeScript) branch too. Please re-open this issue if there are still issues. Thanks! :) |
|
The occurrence is much lower, but still occasionally seeing some unhandled rejections I believe are originating from the library: |
|
Boo-urns... :p Is this in the master (v3) or TypeScript (v4) branch. I’ll look more I to these shortly... |
|
3.0.25 from npm |
|
Here's another I've seen in the logs |
|
I'm curious as to whether this still happens. I'll be working with GitCoin to help reward anyone who would like to help out. Details to follow. :) |
|
I'm going to close this now. In v5 I've tried to keep everything inside an async function that needs to be, so hopefully the resulting Promises will keep this from happening. Please re-open this issue though, or open a new one if you find any instances of an error getting swallowed. Thanks! :) |
Hi,
I've noticed that issues #180 and #185 are due to unhandled promise rejections, so I analyzed the whole codebase (manually and using this eslint plugin) to find the root of these problems.
Note that this problems may be ignorable for now, but in the future node will kill the entire app if a promise rejection is unhandled. See: https://nodejs.org/dist/latest-v8.x/docs/api/deprecations.html#deprecations_dep0018_unhandled_promise_rejections
I identified the following
Promises with athen()and nocatch(), nor being returned:Most of them are present in places where HTTP requests are done, and their failures are not managed.
There's also these cases, where a
unhandledRejectionhandled is set, but you may want to fix themanyway to avoid the bad patter:
Finally, these functions look suspicious. They have a
try {} catch{}and athrowinside athen(). Not sure which was the intention, but thethrowfrom thethen()never gets to thecatchclause`:The text was updated successfully, but these errors were encountered: