Skip to content

fix(console): console.assert() logs on failure, not on success#318

Merged
andreiltd merged 1 commit into
bytecodealliance:mainfrom
watilde:fix/console-assert-inverted-condition
Jul 20, 2026
Merged

fix(console): console.assert() logs on failure, not on success#318
andreiltd merged 1 commit into
bytecodealliance:mainfrom
watilde:fix/console-assert-inverted-condition

Conversation

@watilde

@watilde watilde commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

console.assert()'s condition was inverted: if (!condition) return made it stay silent when the assertion failed and print "Assertion failed" when it passed — the opposite of the cited Console spec, whose step 1 is "If condition is true, return."

Also use JS::ToBoolean() instead of Value::toBoolean(): the latter asserts the argument is already a boolean and crashes a debug build (UB in release) on a non-boolean condition such as console.assert(0). The spec coerces condition to a boolean.

Add an e2e test asserting the serve stdout for truthy/falsy and boolean/non-boolean conditions.

console.assert()'s condition was inverted: `if (!condition) return`
made it stay silent when the assertion failed and print "Assertion
failed" when it passed — the opposite of the cited Console spec, whose
step 1 is "If condition is true, return."

Also use JS::ToBoolean() instead of Value::toBoolean(): the latter
asserts the argument is already a boolean and crashes a debug build (UB
in release) on a non-boolean condition such as console.assert(0). The
spec coerces condition to a boolean.

Add an e2e test asserting the serve stdout for truthy/falsy and
boolean/non-boolean conditions.

Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com>

@andreiltd andreiltd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thank you for the fix.

@andreiltd
andreiltd merged commit f5a7adc into bytecodealliance:main Jul 20, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants