Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blueprints: Throw on failure #982

Merged
merged 12 commits into from
Jan 30, 2024
Merged

Blueprints: Throw on failure #982

merged 12 commits into from
Jan 30, 2024

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jan 30, 2024

Description

Ensures the Blueprint steps throw an error on failure. For now, the error is merely reported in the console. In the future, there may be a nice UI for this.

This PR is an addition to #605 as it prepares the Blueprint steps for the changes in the Blueprint compilation engine.

Detecting a runPHP failure required adjusting the php_wasm.c code to return the correct exit code. The previous method of inferring the exit code was replaced by simply returning EG(exit_status) which is populated by the zend engine.

Furthermore, this PR ships additional unit tests for some Blueprint steps to ensure they indeed throw as expected.

Changes description

throwOnError option

BasePHP.run() now accepts a throwOnError option that throws an error whenever PHP returns an exit code different than 0:

// This will throw a JavaScript exception 
await php.run({
	throwOnError: true,
	code: '<?php no_such_function()'
});

This happens in the following cases:

  • syntax error
  • undefined function call
  • fatal error
  • calling exit(1)
  • uncaught PHP exception
  • ...probably some more

This option is set to true by all the Blueprint steps.

Remaining work

  • Rebuild all the PHP.wasm versions

Testing instructions

This PR comes with test coverage so confirm all the tests pass in CI.

Also, apply this PR locally and visit the following URL:

http://localhost:5400/website-server/#{%22steps%22:[{%22step%22:%22runPHP%22,%20%22code%22:%22%3C?php%20no_such_fn();%22}]}

It should reveal useful error information in the console.

@adamziel
Copy link
Collaborator Author

E2E failures come from #985. This PR merely surfaces the error that's already in there.

This commit ensures the Blueprint steps will throw an error when they
fail. For now, the error is merely reported in the console. In the future,
there may be a nice UI for this.

Detecting a runPHP failure required adjusting the php_wasm.c code to return
the correct exit code. The previous method of inferring the exit code was
replaced by simply returning `EG(exit_status)` which is populated by the
zend engine.

Furthermore, this PR ships additional unit tests for some Blueprint
steps to ensure they indeed throw as expected.

 ## Testing instructions

This PR comes with test coverage so confirm all the tests pass in CI.

Also, apply this PR locally and visit the following URL:

http://localhost:5400/website-server/#{%22steps%22:[{%22step%22:%22runPHP%22,%20%22code%22:%22%3C?php%20no_such_fn();%22}]}

It should reveal useful error information in the console.
@adamziel adamziel force-pushed the surface-blueprint-failures branch from da44dd9 to 4b112c5 Compare January 30, 2024 17:37
@adamziel adamziel merged commit e74da08 into trunk Jan 30, 2024
5 checks passed
@adamziel adamziel deleted the surface-blueprint-failures branch January 30, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant