fix(fs): compiled AbortSignal (#985) + fd-op/chown error codes (#986) match interpreter#1011
Merged
Merged
Conversation
The compiled fd table already threw $NodeError("EBADF") for a stale fd, and
FsChownSync/FsLchownSync threw $NodeError("ENOSYS") on Windows — but both are
thrown inside EmitWithFsErrorHandling's try block, whose catch-all routes every
exception through ThrowNodeError. That helper maps by .NET exception *type* and
fell through to the EINVAL default for any $NodeError, clobbering the deliberate
code (and producing a doubled message).
Fix: ThrowNodeError now detects an incoming $NodeError and preserves its own
code/syscall/path and pre-formatted message verbatim, mirroring the interpreter's
WrapFsOperation (which rethrows a NodeError as-is). One change fixes both the
EBADF and ENOSYS divergences; also cleans up the readlink EINVAL message.
Tests: tightened Fs_CallbackFd_BadFd_And_Chown_Invoke (badfd now asserts EBADF);
added Fs_SyncFdOps_BadFd_ReturnEBADF (fstat/ftruncate/read/write/close) and
Fs_SyncChownLchown_Windows_ReturnENOSYS. Byte-identical interp==compiled.
…d receiver (#985) Compiled AbortSignal is a plain dictionary whose methods are static $Runtime helpers; the typed AbortSignalEmitter strategy only fires when the receiver's static type is AbortSignal. When a signal flows through an `any` parameter (the common case — stdlib facades, user helpers), dispatch fell to generic dynamic member access: addEventListener resolved to undefined ('undefined is not a function'), and `signal.onabort = cb` wrote a plain 'onabort' dict key that FireAbortEvent (reads '_onabort') never saw, so the handler never fired. Fix mirrors the existing GetProperty signal-property fallback (#224): - GetProperty: the signal branch now also returns $TSFunction wrappers for addEventListener/removeEventListener/throwIfAborted (target=null, _expectsThis=true via a '__this' first param), so InvokeMethodValue injects the receiver — making the methods callable from any context. - SetProperty: route `onabort` on a signal dict to AbortSignalSetOnAbort (the internal '_onabort' slot) instead of storing a dead 'onabort' key. - New AbortSignal{AddEventListener,RemoveEventListener,ThrowIfAborted}This '__this'-first wrappers adapt to the existing helpers. All gated on UsesAbortController and the '_reasonSet' slot; standalone preserved (pure emitted IL, no SharpTS.dll dependency). This also makes fs.promises.watch parked-abort terminate promptly in compiled mode (#975 follow-up): the facade's addEventListener('abort', finish) now fires. Tests: dynamic-receiver onabort/addEventListener/removeEventListener/ throwIfAborted/aborted+reason in AbortControllerTests; deterministic parked-abort case added to Fs_PromisesWatch_AsyncIteratorTerminatesOnAbort. Byte-identical interp==compiled. Full suite 14496/0; TS conformance 31/0.
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.
Fixes the two remaining interp↔compiled divergences from the
fsepic (#968): both are compiled-only bugs where the compiled path diverged from the (correct) interpreter. Each is its own commit.#986 — compiled fd-op + chown error codes
A stale file descriptor reported
EINVAL(compiled) instead ofEBADF(interp/Node), andchown/lchownon Windows reportedEINVALinstead ofENOSYS.Root cause (one bug, two symptoms): the fd table's
Get/Closealready threw$NodeError("EBADF", …), andFsChownSync/FsLchownSyncalready threw$NodeError("ENOSYS", …)on Windows — but both are thrown insideEmitWithFsErrorHandling's try block, whose catch-all routes every exception throughThrowNodeError. That helper maps by .NET exception type and fell through to theEINVALdefault for any$NodeError, discarding the deliberate code (and doubling the message).Fix:
ThrowNodeErrornow detects an incoming$NodeErrorand preserves its own code/syscall/path and pre-formatted message verbatim, mirroring the interpreter'sWrapFsOperation(which rethrows aNodeErroras-is). One change fixes both halves; also cleans up the doubled message on the readlinkEINVALpath.#985 — compiled AbortSignal listener API +
onaborton anany-typed receiversignal.addEventListener('abort', cb)threwTypeError: undefined is not a function, andsignal.onabort = cbnever fired — but only when the signal flowed through ananyparameter (the common case: stdlib facades, user helpers). At a statically-typed call site it worked.Root cause: compiled
$AbortSignalis a plainDictionarywhose methods are static$Runtimehelpers, dispatched by the typedAbortSignalEmitterstrategy — which only fires when the receiver's static type isAbortSignal. Through ananyparam, dispatch fell to generic dynamic member access:addEventListenerresolved toundefined, andonabort = cbwrote a dead"onabort"dict key thatFireAbortEvent(which reads the internal"_onabort"slot) never saw.Fix (mirrors the existing dynamic GetProperty signal-property fallback, #224):
GetProperty: the signal branch now also returns$TSFunctionwrappers foraddEventListener/removeEventListener/throwIfAborted(target=null,_expectsThis=truevia a__thisfirst param), soInvokeMethodValueinjects the receiver — the methods become callable from any context.SetProperty: routeonaborton a signal dict toAbortSignalSetOnAbort(the internal slot) instead of storing a dead key.AbortSignal{AddEventListener,RemoveEventListener,ThrowIfAborted}This__this-first wrappers adapt to the existing helpers.All gated on
UsesAbortControllerand the_reasonSetslot; standalone preserved (pure emitted IL, noSharpTS.dlldependency).As a follow-up benefit,
fs.promises.watchparked-abort now terminates promptly in compiled mode (#975): the facade'saddEventListener('abort', finish)finally fires. Updated the stale facade comment accordingly.Tests
Fs_CallbackFd_BadFd_And_Chown_Invoke(badfd now assertsEBADF); addedFs_SyncFdOps_BadFd_ReturnEBADF(fstat/ftruncate/read/write/close) andFs_SyncChownLchown_Windows_ReturnENOSYS(Windows-gated — Unix has a documented compiled no-op).Fs_PromisesWatch_AsyncIteratorTerminatesOnAbort.Verification
dotnet test(full suite): 14496 / 0UsesAbortController/fs paths ECMA never exercises; not baked into the baseline)SharpTS.dllpresentCloses #985
Closes #986