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

Report on unhandled promises #80

Closed
sindresorhus opened this issue Oct 18, 2015 · 8 comments
Closed

Report on unhandled promises #80

sindresorhus opened this issue Oct 18, 2015 · 8 comments
Labels
enhancement new functionality

Comments

@sindresorhus
Copy link
Member

Promises are silent by default and it's easy to forget handling a promise somewhere down the chain or forget to return the promise in the test. Using loud-rejection, we can throw in those cases so the user know they did something wrong.

Need to wait on sindresorhus/loud-rejection#1.

Note to self: Document this in the readme too.

@sindresorhus sindresorhus added the enhancement new functionality label Oct 18, 2015
@sindresorhus sindresorhus changed the title Report unhandled promises Report on unhandled promises Oct 18, 2015
@LinusU
Copy link

LinusU commented Oct 19, 2015

You are actually already using this, which funnily enough breaks the tests that I'm trying to write for loud-rejection 😆

Ava is built with meow which includes loud-rejection.

Man was it annoying to track down as well,

const org = process.on;
process.on = function () {
    console.trace(arguments);
    org.apply(this, arguments);
}

const fn = require('./');
const test = require('ava');
fn();

function fire() {
    return new Promise(function (resolve, reject) {
        setImmediate(function () {
            const org = console.error
            console.error = function () {
                console.error = org
                console.trace(arguments) // <------ Finally manage to find it with this
            }
            reject(new Error('unicorn'));
        });
    });
}

@LinusU
Copy link

LinusU commented Oct 19, 2015

Another fun fact, calling console.trace recursively will run out of memory before hitting max recursion. Found this out the hard way...

<--- Last few GCs --->

    5200 ms: Mark-sweep 1391.4 (1505.2) -> 1172.0 (1220.2) MB, 35.6 / 0 ms [allocation failure] [GC in old space requested].
    5218 ms: Mark-sweep 1172.0 (1220.2) -> 1172.0 (1212.2) MB, 18.1 / 0 ms [allocation failure] [GC in old space requested].
    5238 ms: Mark-sweep 1172.0 (1212.2) -> 1171.7 (1211.2) MB, 19.8 / 0 ms [last resort gc].
    5252 ms: Mark-sweep 1171.7 (1211.2) -> 1171.7 (1211.2) MB, 13.7 / 0 ms [last resort gc].


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x3ec84df37399 <JS Object>
    1: replace(aka replace) [native string.js:~146] [pc=0x230e9b6af485] (this=0x23a9be604101 <Very long string[151036881]>,N=0x3ec84dfb84b1 <JS RegExp>,O=0x3b956bc4e539 <String[2]: \'>)
    2: 0x53c78d8eaf9 <Symbol: 224142380 <String[14]: Symbol.replace>> [/Users/linus/coding/loud-rejection/node_modules/ava/node_modules/babel-core/node_modules/core-js/modules/$.fix-re-wks.js:~15] [pc=0x230e9ae9...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory
[1]    6026 abort      node_modules/.bin/ava

@sindresorhus
Copy link
Member Author

Oh, damn... Although, in the next version of AVA we're spawing each test file in it's own process, so we'll have to explicitly include it then. Try using AVA master as dependency for now. Example: https://github.com/sindresorhus/got/blob/f4cf6c7b44d047d505ed1fdf3a4e361b743bdfc2/package.json#L64

@LinusU
Copy link

LinusU commented Oct 19, 2015

Hmm, for some reason using master stopped my process.on('exit', ...) handler from ever firing... Any idea why?

@LinusU
Copy link

LinusU commented Oct 19, 2015

Also, I can't get rejectionHandled to fire :( tearing my hair out...

@sindresorhus
Copy link
Member Author

@LinusU Maybe just use tape for testing that module then.

@LinusU
Copy link

LinusU commented Oct 19, 2015

I think I got everything working, except for the rejectionHandled... Let's continue the discussion in that repo

@jamestalmage
Copy link
Contributor

Closed via: c5d02f1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new functionality
Projects
None yet
Development

No branches or pull requests

3 participants