Make ParseError checking more clear

[alextes]

JSON test 'should have statusCode in err' requires an instance of
ParseError to compare to but has no opts or data to pass. This commit
makes those arguments optional allowing for empty ParseError
construction.

[floatdrop]

Why not pass those dummy values to ParseError in test?

[alextes]

That would be much nicer @floatdrop!
I'd much rather have the test explicitly use less-sensible arguments than making the actual code more lenient by making the less-sensible arguments defaults.

I didn't do this straight off because I didn't know it was possible. Looking at the ava docs for .throws:

error can be an error constructor, error message, regex matched against the error message, or validation function.

Regardless, the following works:

const err = await t.throws(got(`${s.url}/non200-invalid`, {json: true}), 
	got.ParseError(new Error(), 500, {}, ''));

But to my understanding that's passing an instance, not any of the above, and to make things worse, once I comply with the linting rules and use the 'new' keyword things stop working and I get confused.

Started digging around in the node source, and played around with argument binding, but neither have worked out yet and sadly I don't have more time this morning. I'll try and continue this soon!

Thanks for the suggestion @floatdrop.

[sindresorhus]

Maybe this would work, or if not, we could always just check the individual properties:

const err = await t.throws(got(`${s.url}/non200-invalid`, {json: true}));
t.deepEqual(err, got.ParseError(new Error(), 500, {}, ''));

[alextes]

I figured out why it worked without the new operator. It's because the custom error constructor returns undefined in that case. Which if you're interested fails this check which means node's assert throws nothing, and ava returns the result obtained here which is the HTTPError which also has a property statusCode set to 500.

Now that I understand some of the internals of .throws in NodeJS and AVA that AVA PR we talked about in #267 should quickly follow 😁.

Errors are notoriously inaccessible, you can't iterate them, I tried anyway, but deepEqual is out.

I really like @sindresorhus 's suggestion to just check props then, the way I see it that means we have three okay options. Every error should have a constructor, a message, and a name. That would respectively make the output when some non ParseError was thrown:
Using constructor:

Using message:

Using name:

I like constructor best because I know it's a solid way to tell if two instances come from the same object. Visually, using constructor sucks and comparing name seems most clear to me.

Sorry for taking so long to get back to you two. I'd say it's your call!

[sindresorhus]

Why not just t.is(err.constructor, got.ParseError);?

[alextes]

Fair question. Too long ago to remember what the issue was. Fairly sure I tried. But maybe I wrongly assumed t.is would run into the same problem as t.throws. I'll try tonight!

[alextes]

You were super right. That works fine. Updated the PR.

[sindresorhus]

👍