About error handling in custom module loader

[Icemic]

Hello, I want to know that how to deal with errors (such as invalid path, file busy, etc.) inside module loader.

Since it is defined as

typedef JSModuleDef *JSModuleLoaderFunc(JSContext *ctx, const char *module_name, void *opaque);

is it right to return nullptr when some error occurs? I've tried before calling JS_LoadModule, which returned a promise rejection as expected.

And what about JSModuleNormalizeFunc?

[saghul]

is it right to return nullptr when some error occurs?

Yes, you should throw an exception and return NULL. I recently added some error handling for similar cases (when setting the meta object fails): ab48ede

And what about JSModuleNormalizeFunc?

Same thing. The default one can also fail in case of memory errors for example, and it will simply return NULL with the OOM exception thrown.

[saghul]

I took the liberty to convert this to a discussion.

[Icemic]

Thank you for the answer. It seems I did the correct implementation.

In addition, I noticed that in JS_LoadModuleInternal, we just check whether the module loader returns a NULL and go to fail without a JS_ThrowError.

This lead to a..(eh, UB? I dont know) problem, since JS_GetException now returns JS_UNINITIALIZED instead of JS_UNDEFINED in recent changes. In my previously mentioned trying, I though obtained a promise rejection, but its value is a JS_UNINITIALIZED (typeof value === 'unknown').

So I think it maybe need to review those cases caused by this change, or try to add the missing THROWs (feels difficult to collect all)?

[saghul]

In addition, I noticed that in JS_LoadModuleInternal, we just check whether the module loader returns a NULL and go to fail without a JS_ThrowError.

The called function should be the one throwing the exception, then returning NULL.

[Icemic]

Got it, thanks!