Skip to content

Exceptions overhaul#1069

Merged
xeioex merged 9 commits into
nginx:masterfrom
xeioex:exceptions_overhaul
Jun 3, 2026
Merged

Exceptions overhaul#1069
xeioex merged 9 commits into
nginx:masterfrom
xeioex:exceptions_overhaul

Conversation

@xeioex
Copy link
Copy Markdown
Contributor

@xeioex xeioex commented Jun 3, 2026

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.

  • Wrong receiver and wrong user-visible API usage are TypeError.
  • Numeric/status bounds violations are RangeError.
  • Host/runtime failures remain InternalError.
  • Allocation-only failures use memory errors / OutOfMemory.
  • Correctness fixes are split from pure exception-class retyping when the split is small and clear.

Correctness fixes split out

  • Fetch QuickJS conversion handling:
    • preserve exceptions already raised by QuickJS conversions;
    • add JS_HasException() / JS_ThrowOutOfMemory() guards where ngx_qjs_string() can fail silently on pool allocation;
    • use JS_GetOpaque() instead of JS_GetOpaque2() where class mismatch is a normal miss, avoiding stale pending exceptions;
    • free the headers init value before throwing for non-object Headers.
  • HTTP njs handlers:
    • set pending exceptions before returning NJS_ERROR from response output paths such as sendHeader(), send(), and finish().
  • Stream variables:
    • clear vv->valid and set memory error when stream variable storage allocation fails.

Exception-class changes

  • Fetch, HTTP, Stream, XML, and common helper code now use TypeError for receiver/API misuse.
  • Fetch/HTTP/Stream status and code range checks use RangeError.
  • XML parse failures use SyntaxError.
  • Internal nginx/output/body collection failures remain InternalError.
  • ngx_js_ext_log() / ngx_qjs_ext_log() missing external receiver/context is treated as TypeError, matching the wrong-receiver policy.
  • Stream s.done() while filtering is treated as TypeError, matching other wrong-phase user API calls.

Deliberate non-goals

  • Shared dictionary exception alignment is not included as it requires additional work
  • Existing generic conversion messages in HTTP/Stream handlers are mostly kept to avoid unnecessary user-visible churn.
  • No extra minor receiver tests were added just to exercise every retyped branch.

xeioex added 9 commits June 2, 2026 19:00
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.
Copy link
Copy Markdown
Contributor

@VadimZhestikov VadimZhestikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two suggestions, otherwise looks good:

  1. 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.
  2. 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.

@github-project-automation github-project-automation Bot moved this from New to In Progress in NGINX OSS Unified Workspace Jun 3, 2026
@xeioex
Copy link
Copy Markdown
Contributor Author

xeioex commented Jun 3, 2026

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.

Is fixed by 16f8534.

@xeioex xeioex merged commit 0bdc821 into nginx:master Jun 3, 2026
2 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in NGINX OSS Unified Workspace Jun 3, 2026
@xeioex xeioex deleted the exceptions_overhaul branch June 3, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants