feat: turn on exceptions

[matthewkeil]

A JavaScript Value is a Reference that contains an Address to a value (little "v" actual data) that is stored either on the stack or in the heap. A large part of what the Isolate (instance of a JS virtual machine) does is manage memory and garbage collection of the values (little "v", think data, and big "V" think reference to the Address that points to the data).

These two layers of indirection allow for the data to be tracked/moved/managed/deleted etc by the runtime.

When a v8::Value is requested by a function call the Isolate does a lookup and dereferences the value (little "v") and then returns a Reference to for the value (little v) as a v8::Value (big V). It does so for ref counting which informs the garbage collection system of when a value (both big and little v) can be deleted.

The sticky wicket comes in that if exceptions are turned on, requesting a v8::Value will throw an error if the value (little v) does not exist. With exceptions off it essentially returns a std::optional which in JS world is called a v8::Maybe. The extra steps required without exceptions is a Maybe needs to be checked if it has a value (little v) associated or if it is Empty which is sort of like a nullptr. If an attempt is made to use an Empty value there will be undefined behavior or worse a segfault.

Generally this does not happen because there are several checks that occur in the workflow under the hood. However the one that is most critical is if memory is not allocated correctly for the value (little v) the system will think it is a valid v8::Value but it will be Empty which is.... not... ideal...

There are other considerations as well with regards to errors in JS propagating back into native code, however our codebase does not enter this territory so I will not elaborate.

Turning on exceptions is the preferred way of handling this and is something the docs advocate for. This was something that slipped by me when we worked on this before and I only caught it recently. If we do not want to turn on exceptions there will need to be code added to check and unwrap the Maybe values to turn them in to proper Values.

It will not affect the c-kzg code, its strictly to protect from v8 engine issues.

Here is the doc about exception handling:
https://github.com/nodejs/node-addon-api/blob/main/doc/error_handling.md

And the subsection about code compliance with exceptions turned off:
https://github.com/nodejs/node-addon-api/blob/main/doc/error_handling.md#handling-errors-with-maybe-type-and-c-exceptions-disabled

[matthewkeil]

@jtraglia @asn-d6 I think this is the correct value here but would love a double check. There is also an option for SyncCThrow but the docs recommend Sync.

https://learn.microsoft.com/en-us/visualstudio/msbuild/cl-task?view=vs-2022
https://learn.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-170

[jtraglia]

It sounds like Sync is the most common option and is probably what we should use.

[matthewkeil]

I also think that is the correct value because there are no async C code paths that are floating around... god bless single-threaded JS 😃

[jtraglia]

This is a bit over my head, but I'm going to trust your judgement on this one. Your changes to the bindings.gyp are consistent and should work for all platforms 👍

[g11tech]

lgtm