Fix Error constructors to return rather than throw

[RageKnify]

On master let e = new Error('test'); doesn't work and results in an uncaught error, this PR fixes that by making it so the Error constructors aren't seen as throwing their return value by boa.

Originally opened because SyntaxError.toString and Error.toString were using Value::display while all other error types use Value::to_string.
I don't think this has any effect right now, but I think the errors should all conform to the same pattern.

Has an example you can see TypeError's implementation below:

boa/boa/src/builtins/error/type.rs

Lines 58 to 63 in e83a03a

	pub(crate) fn to_string(this: &Value, _: &[Value], ctx: &mut Context) -> Result<Value> {
	let name = this.get_field("name").to_string(ctx)?;
	let message = this.get_field("message").to_string(ctx)?;
	Ok(Value::from(format!("{}: {}", name, message)))
	}

It changes the following:

• Change error constructors to wrap their return in Ok() rather than Err()
• Change conversion of name and message to use Value::to_string rather than Value::display in SyntaxError.toString
• With the new Builin patterns Error.prototype.toString is only defined once, for the generic Error and all its "subclasses" use that implementation, I've made that version spec compliant.

[codecov]

Codecov Report

Merging #749 into master will increase coverage by 0.07%.
The diff coverage is 79.16%.

@@            Coverage Diff             @@
##           master     #749      +/-   ##
==========================================
+ Coverage   59.13%   59.21%   +0.07%     
==========================================
  Files         155      155              
  Lines        9838     9857      +19     
==========================================
+ Hits         5818     5837      +19     
  Misses       4020     4020              

Impacted Files	Coverage Δ
boa/src/context.rs	70.13% <ø> (ø)
boa/src/builtins/error/mod.rs	66.66% <75.00%> (+23.18%)	⬆️
boa/src/builtins/error/range.rs	65.00% <100.00%> (ø)
boa/src/builtins/error/reference.rs	65.00% <100.00%> (ø)
boa/src/builtins/error/syntax.rs	65.00% <100.00%> (ø)
boa/src/builtins/error/type.rs	65.00% <100.00%> (ø)
boa/src/value/operations.rs	44.67% <0.00%> (+0.07%)	⬆️
boa/src/builtins/json/mod.rs	82.43% <0.00%> (+2.43%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e60d081...d4b9824. Read the comment docs.

[HalidOdat]

This should now be unblocked since #722 has been merged :)

[RageKnify]

@HalidOdat on Discord we discussed using ? when invoking construct_type_error since it will now return result, but in situations where I know it won't Error, becuase I'm giving &str for the message, should I use expect instead? Using ? might generate unnecessary code.

There are some places where I'm forced to use expect because I'm inside closures which don't return Result.

[HalidOdat]

@HalidOdat on Discord we discussed using ? when invoking construct_type_error since it will now return result, but in situations where I know it won't Error, becuase I'm giving &str for the message, should I use expect instead? Using ? might generate unnecessary code.

There are some places where I'm forced to use expect because I'm inside closures which don't return Result.

If we know that it construct_type_error will not give use a Err, then we can return Value and call .expect in the construct_type_error function.

[RageKnify]

Ok, that's my opinion too, there's no need for ? if we know it's safe to use expect, I'll make sure all my changes use expect where possible.

[Razican]

Ok, that's my opinion too, there's no need for ? if we know it's safe to use expect, I'll make sure all my changes use expect where possible.

The idea here is that if at any point in our code, we should never ever reach a branch, it's good to panic. Wether that branch is an Err(), a None or their opposites (or other things, of course).

A panic should mean "we messed up, this is a bug in Boa 100% for sure", and no JavaScript code should ever cause a panic (we still have to solve this part).

But in this case, I think expect() is the way to go in many places.