Skip to content

[v1][breaking] Propagate ctx into polyscript.New[E] and the three FromXxxLoader constructors #145

@robbyt

Description

@robbyt

Summary

Follow-up to #89 (PR #144). #89 added ctx context.Context as the first arg to Compiler.Compile, Loader.GetReader, and script.NewExecutableUnit, but stopped one layer short of the public consumer surface. The three FromXxxLoader constructors and the top-level polyscript.New[E] still don't accept a ctx, so they inject context.Background() inline when calling NewExecutableUnit:

// engines/extism/new.go:102
execUnit, err := script.NewExecutableUnit(context.Background(), cfg.handler, ...)

Same pattern in engines/risor/new.go and engines/starlark/new.go. The ctx-threaded path through Loader.GetReader(ctx) and Compiler.Compile(ctx, reader) ends up exercising a never-cancellable Background() for every caller using the documented public API.

What this means in practice

The HTTP-loaded compile problem #89 set out to solve — "compile can hang indefinitely because nothing reaches for the cancellable variant" — is not actually fixed for the public consumer surface. We moved the context.Background() injection point from inside the loader to the caller side of NewExecutableUnit, but nobody using polyscript.New[E] or FromExtismLoader/FromRisorLoader/FromStarlarkLoader can supply a cancellable ctx today.

Fixes 1 and 2 from #89 (Extism's stored-ctx field, Risor's hard-coded Background() in the inner parser) are real and complete. Fix 3 (HTTP cancellation) is only reachable by callers who drive script.NewExecutableUnit directly — a building-block API that the docs steer people away from.

Proposal

Add ctx context.Context as the first arg to:

  • polyscript.New[E](ctx, source, opts...) — top-level generic constructor
  • extism.FromExtismLoader(ctx, ldr, opts...)
  • risor.FromRisorLoader(ctx, ldr, opts...)
  • starlark.FromStarlarkLoader(ctx, ldr, opts...)

Each forwards its ctx straight into script.NewExecutableUnit. The three context.Background() injections in the engine new.go files go away.

Files

  • polyscript.goNew[E] signature
  • engines/{extism,risor,starlark}/new.goFromXxxLoader signatures + drop the inline context.Background()
  • deprecated.go — the legacy FromRisor*/FromStarlark*/FromExtism* constructors (decide: also take ctx, or leave on the Background() shim until [v1][breaking] Remove deprecated top-level constructors (deprecated.go) #104 removes them)
  • Every example under examples/ — pass ctx
  • All consumer tests — pass t.Context() or equivalent

Acceptance

  • grep -rn 'context\.Background()' --include='*.go' . finds no production hits in the public construction path (the inline injections in engines/{extism,risor,starlark}/new.go are gone)
  • A user can supply ctx, cancel := context.WithTimeout(...) to any public constructor and have the cancel actually reach Loader.GetReader/Compiler.Compile
  • Spot-check: cancelling a ctx during an HTTP-loaded compile actually halts the fetch

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions