Exceptions overhaul#1069
Merged
Merged
Conversation
Wrong receiver errors are caused by JS API misuse, not by internal host failures. Report TypeError for TextEncoder, TextDecoder, console, and fs Stats receiver checks.
Preserve exceptions raised by QuickJS conversions and synthesize an out of memory exception only for silent pool allocation failures in ngx_qjs_string() callers. Use JS_GetOpaque() where a class mismatch is a normal miss, avoiding a stale pending TypeError while continuing with non-Headers and non-Request input. Also release the headers init value before throwing for a non-object Headers option.
Report API misuse in Fetch, Request, Response, and Headers as TypeError, and report status bounds violations as RangeError. Keep internal host failures as InternalError and preserve conversion helper exceptions where they already provide the real cause.
Set pending exceptions before returning NJS_ERROR from njs response output paths. Previously sendHeader(), send(), and finish() could fail without an exception being available to the VM.
Report HTTP request API misuse as TypeError and status bounds violations as RangeError. Keep nginx output and request body collection failures as InternalError, since they are host/runtime failures rather than invalid JavaScript arguments.
Reset the variable value validity flag when allocating storage for a stream variable fails. This matches the HTTP variable path and prevents a failed assignment from leaving a half-initialized value marked as valid.
Report stream session API misuse as TypeError, including wrong receiver, unknown or duplicate event handlers, wrong handler phase, and invalid variable access. Report status bounds violations as RangeError and keep stream output pipeline failures as InternalError. Also fix the async send error message spacing.
Report XML API misuse as TypeError and XML parse failures as SyntaxError, aligning njs and QuickJS behavior. The QuickJS message "'this' is not XMLNode or XMLDoc" is changed to "value is not XMLNode or XMLDoc" because qjs_xml_node() is not always validating this.
Align common helper failures with the exception policy used by both engines: invalid numeric conversion is TypeError, and missing external receiver/context for common log helpers is TypeError rather than an internal host failure. The numeric conversion message changes from "is not a number" to "invalid number" to match the QuickJS helper text.
VadimZhestikov
approved these changes
Jun 3, 2026
Contributor
VadimZhestikov
left a comment
There was a problem hiding this comment.
Two suggestions, otherwise looks good:
- QJS/njs asymmetry (commit 3): ngx_qjs_request_ctor uses JS_ThrowInternalError for "input is not string or a Request
object" where the njs path uses njs_vm_type_error. Should be JS_ThrowTypeError. - Minor classification inconsistency (commit 5): "failed to convert uri arg" / "failed to convert options.method"
etc. are labeled TypeError but the conversion failure at those points is almost always OOM (the value type was
verified earlier). Removing these explicit messages (as done elsewhere in the PR) would be cleaner and let njs set the
right error type.
Contributor
Author
Is fixed by 16f8534. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleaning up of exception classes for QuickJS/njs integration code + small fixes found. The goal is not to rewrite
all messages, but to make exception classes predictable while keeping real bug fixes visible for review and backports.
TypeError.RangeError.InternalError.OutOfMemory.Correctness fixes split out
JS_HasException()/JS_ThrowOutOfMemory()guards wherengx_qjs_string()can fail silently on pool allocation;JS_GetOpaque()instead ofJS_GetOpaque2()where class mismatch is a normal miss, avoiding stale pending exceptions;Headers.NJS_ERRORfrom response output paths such assendHeader(),send(), andfinish().vv->validand set memory error when stream variable storage allocation fails.Exception-class changes
TypeErrorfor receiver/API misuse.RangeError.SyntaxError.InternalError.ngx_js_ext_log()/ngx_qjs_ext_log()missing external receiver/context is treated asTypeError, matching the wrong-receiver policy.s.done()while filtering is treated asTypeError, matching other wrong-phase user API calls.Deliberate non-goals