[js-api] Throw RangeError when getArg's index is out of bounds

[aheejin]

Closes #184.

[Ms2ger]

Sure. Do you want to update the "Getting out-of-range argument" subtest in getArg.tentative.any.js as well?

[aheejin]

@thibaudmichaud Is this change OK in the V8 implementation of JS API?

[thibaudmichaud]

Yes, this change should be straightforward.

[aheejin]

@thibaudmichaud Has this been implemented? I ran this on https://github.com/web-platform-tests/wpt and it is failing:

Running 3 tests in web-platform-tests

  ▶ Unexpected subtest result in /wasm/jsapi/exception/getArg.tentative.any.worker.html:
  │ FAIL [expected PASS] Getting out-of-range argument
  │   → assert_throws_js: function "() => exn.getArg(tag, value)" threw object "TypeError: WebAssembly.Exception.getArg(): Index must be convertible to a valid number" ("TypeError") expected instance of function "function RangeError() { [native code] }" ("RangeError")
  │ 
  │     at Test.<anonymous> (http://web-platform.test:8000/wasm/jsapi/exception/getArg.tentative.any.js:46:5)
  │     at Test.step (http://web-platform.test:8000/resources/testharness.js:2595:25)
  │     at test (http://web-platform.test:8000/resources/testharness.js:628:30)
  └     at http://web-platform.test:8000/wasm/jsapi/exception/getArg.tentative.any.js:26:1

  ▶ Unexpected subtest result in /wasm/jsapi/exception/getArg.tentative.any.html:
  │ FAIL [expected PASS] Getting out-of-range argument
  │   → assert_throws_js: function "() => exn.getArg(tag, value)" threw object "TypeError: WebAssembly.Exception.getArg(): Index must be convertible to a valid number" ("TypeError") expected instance of function "function RangeError() { [native code] }" ("RangeError")
  │ 
  │     at Test.<anonymous> (http://web-platform.test:8000/wasm/jsapi/exception/getArg.tentative.any.js:46:5)
  │     at Test.step (http://web-platform.test:8000/resources/testharness.js:2595:25)
  │     at test (http://web-platform.test:8000/resources/testharness.js:628:30)
  └     at http://web-platform.test:8000/wasm/jsapi/exception/getArg.tentative.any.js:26:1

Ran 3 tests finished in 8.8 seconds.
  • 1 ran as expected. 1 tests skipped.
  • 2 tests had unexpected subtest results

[thibaudmichaud]

It was implemented, but if the argument cannot be converted to a uint32, we throw a TypeError rather than a RangeError. This seems very similar to table.grow, which also throws a TypeError in this case: https://github.com/WebAssembly/spec/blob/main/test/js-api/table/grow.any.js. Wdyt about changing the getArg spec test for consistency?

I am not sure why we didn't catch that earlier though, we are supposed to include these tests in our test suite, I'll look into it.

[thibaudmichaud]

I didn't see that the test was only added to wpt a few hours ago, so that answers my last concern.

[Ms2ger]

Doh, my mistake in reviewing the test change. It seems like the "Index out of bounds" test above the changed one already expected the new behavior.

[Ms2ger]

#257