[BUG-34] withHookContext error swallowing makes hook tests pass vacuously

[shroy]

Severity: P0 (test integrity — systemic) · Batch: B1 (test-integrity + CI)

Location

src/core/hookContext.js:170-183: withHookContext catches everything → logger.error → return null

Root cause

Seven spec files (useEvents/useClasses/useAttributes/useStyles/useText/signalEffect/nonSignalWarning) place expect() assertions inside withHookContext(...) callbacks — a thrown AssertionError is eaten by the framework and the test passes.

Proof

src/tests/useEvents.spec.js:216-242 asserts addEventListener receives raw handler1 and that signal handlers fire — both demonstrably false in the current implementation (BUG-1), yet the suite is green.

Proposed fix

In tests, assert outside the callback or capture results; better, make withHookContext rethrow unknown errors in a test/debug mode. Re-run the whole suite after fixing — expect real failures to surface.

Verification recipe

Spec: withHookContext(el, () => { expect(true).toBe(false) }) followed by an outside assertion that the callback's failure surfaced — demonstrate it returns null and the test framework saw no failure. Expected on unmodified code: swallowed → confirmed.

[shroy]

Phase 1 verification: CONFIRMED

Method: runtime repro (vitest + jsdom), corroborated by static reading of the source.

Root cause (static)

src/core/hookContext.js:170-183 wraps the callback in tryCatch. Any thrown value hits onError, which logs and returns null, so the exception never propagates:

export const withHookContext = (element, callback) => {
  const context = createHookContext(element)
  hookContextStack.push(context)

  return tryCatch({
    fn: () => {
      const result = callback()
      executeEffectQueue(context)
      return result
    },
    onError: (error) => {
      logger.error('Error in withHookContext:', error)
      return null            // <-- swallows EVERYTHING, incl. AssertionError
    },
    onFinally: () => {
      hookContextStack.pop()
    }
  })
}

tryCatch (src/utils/try-catch.js:33-43) is a plain try/catch that returns onError(error) — no rethrow. A vitest AssertionError thrown by an inner expect() is therefore eaten and the enclosing test passes vacuously.

Repro spec (asserts CORRECT behavior; fails on unmodified code)

import { describe, it, expect, vi } from 'vitest'
import { withHookContext } from '../../core/hookContext.js'

describe('BUG-34: withHookContext error swallowing', () => {
  const element = document.createElement('div')

  it('should NOT swallow an exception thrown inside the callback (should rethrow)', () => {
    const boom = new Error('boom from inside callback')
    expect(() => {
      withHookContext(element, () => {
        throw boom
      })
    }).toThrow('boom from inside callback')
  })

  it('should surface an inner expect() AssertionError to the test framework (recipe from issue)', () => {
    let innerRan = false
    let caught = null
    try {
      withHookContext(element, () => {
        innerRan = true
        expect(true).toBe(false) // intentionally-failing inner assertion
      })
    } catch (err) {
      caught = err
    }
    expect(innerRan).toBe(true)
    expect(caught, 'inner expect() failure was swallowed by withHookContext').not.toBeNull()
  })

  it('demonstrates the swallow: withHookContext returns null and logs instead of throwing', () => {
    const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
    let result
    let threw = false
    try {
      result = withHookContext(element, () => {
        throw new Error('should have propagated')
      })
    } catch {
      threw = true
    }
    errorSpy.mockRestore()
    expect(threw, 'withHookContext should propagate the error rather than swallow it')
      .toBe(true)
  })
})

Decisive output (unmodified code)

All three assertions of correct behavior FAIL, and stderr shows the framework caught the AssertionError from the issue's exact recipe:

stderr | ... > should surface an inner expect() AssertionError ...
[HookTML] Error in withHookContext: AssertionError: expected true to be false // Object.is equality
    at src/tests/__verify__/issue-45.spec.js:45:22
    at fn (src/core/hookContext.js:172:22)
    at Module.tryCatch (src/utils/try-catch.js:35:12)
    at Module.withHookContext (src/core/hookContext.js:170:10)

 ❯ src/tests/__verify__/issue-45.spec.js  (3 tests | 3 failed)
   → expected [Function] to throw an error
   → inner expect() failure was swallowed by withHookContext: expected null not to be null
   → withHookContext should propagate the error rather than swallow it: expected false to be true

 Test Files  1 failed (1)
      Tests  3 failed (3)

Interpretation

withHookContext catches all thrown values (including vitest AssertionError), logs [HookTML] Error in withHookContext: …, and returns null instead of rethrowing. expect() assertions placed inside a withHookContext callback (or any framework-invoked hook/component) are silently eaten, so those tests pass vacuously — exactly as BUG-34 claims. The stack trace routing through hookContext.js:172 → try-catch.js:35 → hookContext.js:170 is direct proof of the swallow. Confirmed.