module: improve error message for invalid data URL

[aduh95]

Fixes: #37647

Changes the error message when importing module with an unknown/unsupported MIME type from:

TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected string to be returned for the "format" from the "loader getFormat" function but got type object.

to

TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "data:application/json,[]" has an unsupported MIME type "application/json"

[targos]

@nodejs/modules

[DerekNonGeneric]

Just my 2¢. Not sure that error code has been used for this purpose historically.

[bmeck]

CC: @ljharb since this mentions MIME to user

[bmeck]

to clarify a bit, these should have been picked up and could have been generated in the past

[ljharb]

Seems fine to me, since anyone using a data URL is already familiar with MIMEs (this is ofc not true for almost every other specifier category).

[aduh95]

Ping @nodejs/modules for reviews.

[DerekNonGeneric]

Aside from my outstanding comments (non-blocking), everything else looks pretty good to me.

I will be digging around this part of codebase in a few days, so I will report back if I find issues.

[guybedford]

Perhaps Unsupported MIME "text/plain" loading "data:text/plain,export default 0".

Also it could be nice if we could support the parent context for tracing where it comes from eg if it were import('data:...') in a module, it helps to know where the import expression is.

[aduh95]

@guybedford Agreed, although I think this would deserve its own PR to align with other ERR_INVALID_MODULE_SPECIFIER uses in core.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37108/

[guybedford]

@aduh95 I do think it's important to move the "unsupported MIME" part to the beginning of the message though. This is because the data URL could be of any length, so that when scanning the error the user should be able to easily see the "unsupported MIME" part first. It also fixes that the current error isn't quite gramatically complete without a comma or colon or something type of break otherwise.

Alternatively to make it gramatically correct:

• Use a full stop and a new sentence - The MIME type "..." is incorrect.
• Use a comma / colon / dash.
• Use "has an unsupported MIME type "...""

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37109/

[aduh95]

I went to the last suggestion as it was the easiest to implement. I think you have a point regarding very long specifiers occulting the reason, maybe we can fix that in a similar way as proposed in #37852 (comment)?

[DerekNonGeneric]

[…] maybe we can fix that in a similar way as proposed in #37852 (comment)?

I believe the feature request outlined in #37581 is precisely the error property we need.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37110/

[aduh95]

Landed in 46062a0