mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 16:02:36 +00:00
d458e54601251996c0b5c8d478bbd39b2f80c19e
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Ruben FiszelandClaude Opus 5
|
785277e0bb |
feat(nativets): bound fetch on a peer that never answers (#11026)
* feat(nativets): bound fetch on a peer that never answers
deno_fetch applies no deadline of any kind. A peer that completes the TCP
handshake, accepts the request and then goes silent leaves `await fetch(...)`
pending indefinitely, holding its worker slot until the *job* timeout -- which
on self-hosted defaults to DEFAULT_SELFHOSTED_TIMEOUT, i.e. 7 days.
Nothing else catches this. Zombie-job detection keys off a stale
v2_job_runtime.ping, and a worker blocked inside a pending fetch keeps pinging
normally throughout: the worker is alive and healthy, only the work is dead.
What this bounds is the wait for a response to begin, and it stops there:
- a peer that never answers -> rejected after N seconds
- a peer slow to answer, but under N -> unaffected
- a body that then streams for an hour,
or is read slowly by the caller -> unaffected, always
That last line rules out the obvious implementation: AbortSignal.timeout(N)
around every fetch would bound the hang and break every streaming response and
long download. This is a hang detector, not a latency budget.
Default 300s via WINDMILL_FETCH_RESPONSE_TIMEOUT_SECS (0 disables), with a
per-script `//fetch_response_timeout <seconds>` annotation alongside the
existing //useragent and //proxy. Both nativets paths inherit it, since
eval_fetch_timeout and the dedicated-worker path in bun_executor both funnel
through create_nativets_runtime.
The ms value is clamped to i32::MAX: deno_web's setTimeout runs its delay
through webidl.converters.long, a 32-bit conversion that *wraps*, so a setting
past ~24.8 days would come out negative and fire immediately -- turning an
over-generous timeout into an instant one on every fetch.
The window covers connect, TLS and request upload as well as server think
time, so a very slow large upload is bounded by it too; the error message says
so rather than claiming the connection went silent.
Not covered: a body that stalls midway. Reaching that needs the response's
InnerBody, which deno_fetch keeps module-private, and every way to wrap it
from outside changes observable Response semantics (locking, bodyUsed,
double-consume errors). Left for a follow-up in deno_fetch itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JshNT5XVH78ZsTMvDWfFHb
* test(nativets): cover the instance-wide response-timeout env var
A typo in WINDMILL_FETCH_RESPONSE_TIMEOUT_SECS would compile, pass every
other test, and silently hand every operator the 300s default -- the same
class of silent-default failure the timeout itself exists to prevent. Its
own test binary, since a LazyLock resolves the value once per process.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JshNT5XVH78ZsTMvDWfFHb
* fix(nativets): inherit the caller's RequestInit, and clear the long-poll ceiling
Two problems with the first cut, both found in review.
`{ ...init, signal }` copied only own enumerable properties, but RequestInit is
a WebIDL dictionary whose members deno_fetch reads with plain property gets
that walk the prototype chain. Anything inherited or non-enumerable was
dropped: `fetch(url, Object.create({method: "POST"}))` silently became a GET.
Worse, a non-object init went from a loud TypeError to a silent GET, because
spreading "POST" yields {0:"P",1:"O",...} -- a valid dictionary with ignored
keys. Now the init is inherited from rather than copied, and a non-dictionary
is handed straight back to deno_fetch for its own TypeError.
The 300s default also sat at half of TIMEOUT_WAIT_RESULT (600s), which
run_wait_result long-polls against with no response headers. A script running
another job synchronously for 300-600s would have timed out client-side while
the server was still legitimately holding the request open -- the long-poll
risk class, instantiated inside the product and reachable without writing a
raw fetch. Default raised to 900s, with the constraint recorded where someone
would break it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JshNT5XVH78ZsTMvDWfFHb
* fix(nativets): hand the caller's RequestInit to Request untouched
Carrying a WebIDL dictionary across by hand has no safe form, and both
previous attempts were wrong in opposite directions. Spreading a copy drops
inherited and non-enumerable members, and turns a non-object init from a loud
TypeError into a silent GET. Inheriting from it via Object.create fixes those
but makes the child the receiver, so an accessor on the original runs against
an object that lacks its private-field brand:
Cannot read private member #body from an object whose class did not
declare it
So don't carry it at all. fetch()'s own first act is `new Request(input,
init)`; doing that here hands the init to the same constructor, read exactly
as it would be without this wrapper, and our signal travels in an init we own.
`req.signal` is then deno's own resolution of init.signal over an input
Request's signal, which removes the hand-rolled version of that rule too.
The Request is built twice as a result, once here and once inside fetch. That
is cheap: cloneInnerRequest carries method, headers, redirect mode, clientRid
and blob entry, and a body is proxied rather than buffered -- a static body is
a shallow {body, consumed} copy sharing its bytes, a stream gets a one-chunk
pass-through.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JshNT5XVH78ZsTMvDWfFHb
* fix(nativets): keep an aborted fetch settling in the same tick
deno_fetch keeps its outer fetch non-async on purpose: "WPT has a test that
aborted fetch is settled in the same tick. This means we cannot wrap the
promise if it is already settled" (26_fetch.js). An `async` wrapper adopts
that promise through another one, so a rejection that used to land before any
microtask queued after the call now lands after it.
Made the wrapper non-async, with an early return that hands deno's settled
rejection straight back for an already-aborted signal, and no timer armed
there since there is no response to wait for. Construction still has to reject
rather than throw, so it is caught and returned as a rejection, which is what
the `async` was buying.
The comment claiming this matched deno_fetch's own `async function fetch` was
wrong on two counts -- that function is not async, and the wrapper was not
matching it. Replaced with the constraint that actually holds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JshNT5XVH78ZsTMvDWfFHb
* fix(nativets): keep fetch's observable shape and reach intrinsics safely
Three ways the wrapper was distinguishable from the fetch it replaces, all
observable from a script sharing the isolate.
`.then` was an ordinary property lookup, so `Promise.prototype.then =
undefined` broke fetch after the request had already gone out. deno's own
modules reach intrinsics through primordials, and this file already captured
setTimeout, clearTimeout and Promise.reject for exactly that reason, so the
lookup was the odd one out. Now captured alongside them.
Declaring `init` without a default made `fetch.length` 2 where the standard
says 1. And the empty-call branch forwarded two explicit `undefined`s, so
deno's required-argument check saw two arguments and raised "Invalid URL:
'undefined'" instead of "1 argument required". Forwarding through
ReflectApply preserves the count.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JshNT5XVH78ZsTMvDWfFHb
* docs: clarify fetch timeout restart requirements
* fix: capture native fetch abort helpers
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Ruben FiszelandClaude Opus 4.8
|
6d1e12d5e9 |
feat(nativets): expose the standard web-platform globals deno_web provides (#10112)
* feat(nativets): expose standard web-platform globals for bun parity Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(nativets): wire bun-present Event subclasses and add construction smoke test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(nativets): seed performance.timeOrigin per isolate, drop broken reportError Addresses CI Codex review on #10112: - performance.timeOrigin was undefined (setTimeOrigin never called); seed it per isolate via __wmInitPerIsolate executed from create_nativets_runtime. - reportError needs a global EventTarget this runtime never installs; drop it. - reword the namespace-import comment to not describe drafting history. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(nativets): wire DOMException global + broad edge-case smoke sweep DOMException is present in bun and, more importantly, deno_web references it as a global: AbortController.abort() with no reason constructs a DOMException("...", "AbortError"), so the already-wired AbortController/ AbortSignal threw "DOMException is not defined" on abort. Surfaced by a new functional edge-case sweep (smoke_web_globals_edge_cases) that exercises every wired global for real (not just presence) — DOMException/abort, AbortSignal.timeout, EventTarget dispatch, stream tee/reader/writer, all 3 compression formats, structuredClone Map/Set/Date/circular/reject-function, performance mark/measure, MessagePort delivery — plus a check that the merged Web Crypto globals still work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(nativets): restore arg-default smoke tests dropped in merge, drop history comments Addresses CI Codex/Pi review on the merge commit: - Merge conflict resolution (checkout --ours) dropped smoke_missing_optional_arg_uses_default and smoke_explicit_null_arg_is_preserved (added on main by #10111); restore them. - Reword edge-case-sweep comments to state the constraint, not how the gaps were found. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(nativets): give reportException a global dispatch target; wire stream reader/controller globals Addresses CI Codex review on #10112: - P1: a throwing EventTarget listener (and reportError) is routed through deno_web's reportException, which dispatches on a saved global reference. With none set, dispatchEvent threw a masking error that hid the original. Wire a dedicated EventTarget as that target so the ORIGINAL error is reported (async unhandled, matching bun). Does NOT make globalThis an EventTarget (bun's isn't either). Re-adds reportError, now functional. Regression test asserts the original error is surfaced, not a masking one. - P2: wire the stream reader/controller globals bun also exposes (ReadableStreamDefaultReader/BYOBReader, ReadableStreamDefault/ByteStreamController, ReadableStreamBYOBRequest, WritableStreamDefaultWriter/Controller, TransformStreamDefaultController) for instanceof parity; sweep verifies via real reader/writer/controller instances. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(nativets): make globalThis an EventTarget so globalThis.reportError() works Addresses follow-up CI Codex review on #10112: - P1: the prior fix saved a *separate* EventTarget as the global reference, so globalThis.reportError() still failed its receiver check (this === globalThis_) with 'Illegal invocation'. Make globalThis itself the saved reference by turning it into a functional EventTarget (setPrototypeOf to DedicatedWorkerGlobalScope + setEventTargetData + webidl brand + saveGlobalThisReference), per isolate in __wmInitPerIsolate. Both reportError(e) and globalThis.reportError(e) now surface the original error (async, matching bun) instead of throwing. New test smoke_report_error_both_call_forms covers both call forms. - P2: reword the regression-test comment to state the invariant, not the patch history. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(nativets): wire performance constructor globals for bun parity Addresses the P2 nit in the CI Codex review: bun exposes Performance, PerformanceEntry, PerformanceMark, and PerformanceMeasure as globals (deno_web exports all four), so wire them alongside the performance singleton. The edge-case sweep verifies instanceof against real mark/measure entries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(nativets): state global-wiring comment as a constraint, not patch history Addresses the P2 in the CI Codex review: reword the block comment to describe the current bun-parity constraint and the deliberate EventSource/ImageData exclusions, without narrating what was or wasn't wired before (per AGENTS.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ruben FiszelandClaude Opus 4.8
|
ba232544e7 |
feat(nativets): add Web Crypto support via deno_crypto (#10109)
The nativets in-process runtime (deno_core) exposed no Web Crypto API:
`crypto` was undefined, so scripts could not use `crypto.getRandomValues`,
`crypto.randomUUID`, or `crypto.subtle`, even though the bun runner provides
them. This closes that parity gap by registering the `deno_crypto` extension
and wiring the crypto globals onto `globalThis`.
- Pin `deno_crypto = "0.223.0"`, the sibling release of the already-pinned
deno_core 0.352 / deno_web 0.240 stack (deps: deno_core ^0.352,
deno_web ^0.240, deno_error =0.6.1), so the rest of the deno stack is
untouched.
- Register `deno_crypto::init(None)` after `deno_web` in both the snapshot
(build.rs) and the runtime (lib.rs) extension lists, keeping the snapshot a
prefix of the runtime list. deno_crypto declares deps = [deno_webidl,
deno_web], which the position satisfies.
- Import `ext:deno_crypto/00_crypto.js` in runtime.js and assign
`crypto` / `Crypto` / `CryptoKey` / `SubtleCrypto` to `globalThis`.
- Add the `smoke_web_crypto` opt-in smoke test asserting the UUIDv4 shape of
`randomUUID`, a non-zero `getRandomValues` fill, and the known
SHA-256("abc") vector via `subtle.digest`.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ruben FiszelandClaude Opus 4.6
|
3f68e3a58e |
refactor: isolate deno_core into windmill-runtime-nativets subcrate (#7848)
* refactor: isolate deno_core into windmill-runtime-nativets subcrate Remove deno_core from flow eval and isolate nativets V8 runtime into a dedicated subcrate so deno_core compilation no longer blocks windmill-worker or windmill-api. - Create windmill-jseval crate: QuickJS-based JS eval for flow expressions and batch rerun, extracted from windmill-worker - Create windmill-runtime-nativets crate: all deno_core/V8 deps and nativets script execution, with build.rs snapshot generation - Simplify windmill-worker: remove all deno_* direct deps, empty build.rs, gate nativets behind optional dep - Update windmill-api: use windmill-jseval for batch rerun instead of deno_core, remove deno_core feature entirely - Add nativets integration tests (nativets_jobs.rs) and parallel stress test (nativets_stress.rs, 8 workers x 200 jobs) - Remove dead code: deno flow eval path, USE_QUICKJS env var, parity tests (replaced with 63 standalone expected-value tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback for deno_core isolation - Deduplicate unsafe_raw() into windmill-common/src/utils.rs (single source) - Delete orphaned runtime.js and windmill-client.js from windmill-worker/src/ - Fix operator precedence in windmill-jseval with explicit parentheses - Remove unnecessary return keyword in heap limit callback - Remove redundant as usize casts - Remove ~150 lines of commented-out code from runtime.js - Remove commented-out #[cfg] in build.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * otel ee --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |