fix: make Go handler binding fail loudly instead of masking errors#350
Merged
Conversation
Removes silent fallbacks in backend handler binding and build-data
resolution that hid the real cause of a failure:
- Compile-error masking (A): when a sibling Go package fails to compile,
binding no longer falls back to an inline go {} block and reports a
misleading bound handler. The action/API/load binding stays
'could not be inspected'; the compile error is reported by
go_package_error.
- Ambiguous handlers (C): a handler declared in both same-package Go and
an inline go {} block now emits the new ambiguous_backend_handler
warning instead of silently preferring one source.
- Swallowed go list errors (B): samePackageImportPath and the
go-bindings report now surface the real 'go list' stderr (e.g. a
missing go.mod) instead of a generic 'requires a buildable Go package'
message.
- Component-script masking (D): a component-script resolution error
during build now fails the build instead of silently omitting a page's
component scripts.
Adds source.BackendBinding.Ambiguous, resolveBackendBinding /
resolveFragmentBinding, and the ambiguous_backend_handler diagnostic
code. Tests cover ambiguity, compile-error non-masking, inline/fragment
unexported near-misses, production-policy fragment behaviour, and the
surfaced go list error.
The remaining swallow-style sites surfaced by an audit are not masking:
malformed g:for is reported by validate_component_lists; the render-time
view re-parse is guarded by view_parse_error; contractscan strconv.Unquote
runs on parser-guaranteed quoted literals; and the LSP feature paths
publish parse errors via CheckSource while gracefully degrading.
…interpolation error
Two more silent-fallback fixes of the same class:
- internal/contractscan/types.go: scanGoListExportFiles used .Output()
and returned the bare error, so a failing 'go list -deps -export'
reached the type checker as an opaque 'load export data for X: exit
status 1'. It now surfaces the go list stderr (the real cause).
- internal/buildgen/build_data_interpolate.go: a missing interpolation
reference always reported 'unknown route param', even for an explicit
field("...") build-data field. It now reports 'unknown build data
field' for field() expressions and 'unknown build data field or route
param' for bare {name} references.
Audited siblings that were NOT changed because they are not masking:
currentAppModule's go list -m only fails outside a module (where the
build can't work anyway); validate_packages goListExportFiles falls back
to importer.Default and go_package_error catches real failures;
cssClassesFromViewBody / view re-parses run only after view_parse_error
validation; layout/component scope-precedence resolution is intentional
shadowing, not error masking; stateValueString json.Marshal can't fail
on parsed .gwdk literals.
…odule replace currentAppModule swallowed any 'go list -m -json' error and returned false, so a broken/missing main module silently produced a generated go.mod without the app module's require/replace. When the generated app imports app-owned packages, that go.mod then fails to build with an opaque 'cannot find package'. currentAppModule now returns an error (with the go list stderr), and moduleSource fails loudly with that cause when the app actually imports app-owned packages (appHasLocalModuleImports / isLocalModuleImportPath). When nothing app-owned is imported, the main module is not needed and the failure is still ignored, so stdlib/gowdk-only apps are unaffected. Tests: isLocalModuleImportPath heuristic and appHasLocalModuleImports. Completes a full re-audit for this bug class. The other audited sites are not masking and were intentionally left: - validate_packages goListExportFiles falls back to importer.Default and real type errors still surface via go_package_error; - defaultCSSInputs already errors on a missing configured input, and cssOutputExcludePattern returns "" only when output is outside the CSS root (filepath.Abs effectively cannot fail); - layout/component same-package-then-global resolution is intentional scope shadowing, not error masking; - strconv.Unquote on AST import literals and stateValueString json.Marshal on parsed .gwdk literals cannot fail; - LSP feature paths degrade gracefully while publishing parse errors via CheckSource.
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
1 participant
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.
Removes silent fallbacks in backend handler binding and build-data resolution that hid the real cause of a failure. Branched clean off `main` (no unrelated work).
Fixes
Mechanics
Adds `source.BackendBinding.Ambiguous`, `resolveBackendBinding` / `resolveFragmentBinding`, and the registered `ambiguous_backend_handler` diagnostic code.
Tests
Cover ambiguity, compile-error non-masking, inline/fragment unexported near-misses, production-policy fragment behaviour, and the surfaced `go list` error. Full suites pass (`compiler`, `buildgen`, `diagnostics`, `source`, `cmd/gowdk`, `lang`); `gofmt`/`vet` clean.
Audit note (no action needed)
The remaining swallow-style sites surfaced by an audit are not masking and were intentionally left: malformed `g:for` is reported by `validate_component_lists`; the render-time view re-parse is guarded by `view_parse_error`; `contractscan`'s `strconv.Unquote` runs on parser-guaranteed quoted literals; and the LSP feature paths publish parse errors via `CheckSource` while degrading gracefully.