How can we override reject creating a new Error Object?

[JemiloII]

sinon/lib/sinon/default-behaviors.js

Line 176 in 4310343

if (typeof error === "string") {

I'm maintaining some code that rejects strings and this is creating issues while testing. my bluebird catches are not behaving correctly.

bluebird.rejects('Something')
    .catch(e => e === 'Something' /* true */, () => console.log(typeof e) /*string*/)
    .catch(() => console.log('should not be here'));

However using sinon.stub().usingPromise('bluebird').rejects('Something'); ends up creating an error object with the name Something. Not expected at all.

Before someone says we are only supporting native Promises, let's investigate what they do!

Promise.reject('Something')
     .catch(e => {
        console.log(e === 'Something'); // true
        console.log(typeof e); // string
    });

Hmmm, they don't create an Error class, they just toss the exception it was given like a throw.

try { 
    throw 'Something'; 
} catch (e) { 
    console.log(e === 'Something');  // true
    console.log(typeof e); // string
}

[mroderick]

Before someone says we are only supporting native Promises, let's investigate what they do!

That's a strawman fallacy. You know well enough that we support promise libraries, as you use them in your example.

The Sinon maintainers are fully aware of how JavaScript promises work, including that Promise.reject() will pass anything you pass to it.

I'm maintaining some code that rejects strings

In my opinion, rejecting promises with String instead of Error is really doing a disservice to Future Developer, who has to debug what we write today. Strings are not very useful for debugging, as they don't capture stack traces. Error on the other hand does. Therefore, I think that rejects() behaviour is desirable, as that makes it easy to use the debugger to quickly find out what went wrong.

It seems you disagree, but the only statement that you've made is that Sinon doesn't meet your expectations. If you think the default behaviour should be changed, then please post arguments for why, keeping in mind that the change has potential to affect many users.

If there are solid arguments for changing the default behaviour, I can't imagine any of the maintainers of Sinon would oppose a pull request to change it.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[dtom90]

I'd say there could be differing opinions as to whether or not a stack trace is actually useful in certain scenarios. In my particular situation, I actually don't care about the stack trace, I only care about the error message and then passing it somewhere else. So I've written code to handle both String and Error type reject reasons. However, since sinon does the String -> Error coercion for me, I cannot test the branch of my code that handles strings. As such I cannot get full coverage

[mantoni]

Stack trace usefulness and discussions about whether one should create error objects aside, I agree that generally

const example = sinon.stub().rejects(something);

should behave the same as

const example = () => Promise.reject(something);

Currently, this is not the case if something is a string. I'd say the current behavior is a bug.

[JemiloII]

I agree.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.