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.

Sign up for GitHub

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

Jump to bottom

REST API: return 500 on script failure #3223

Closed
akamac opened this issue Dec 24, 2020 · 1 comment · Fixed by #3337
Closed

REST API: return 500 on script failure #3223

akamac opened this issue Dec 24, 2020 · 1 comment · Fixed by #3337
Labels
Feature request

Comments

@akamac
Copy link

akamac commented Dec 24, 2020

Hi there,

I would like to expose a script to serve as a Kubernetes readiness probe, but noticed that all API calls return 200 whether the script fails or not. Can we handle it in a more appropriate manner?
Meanwhile, I'm just using exec probe.

Thanks!
@renecannao
Copy link
Contributor

This seems a valid issue.

JavierJF added a commit that referenced this issue Mar 10, 2021
@JavierJF
Closes #3223: Major rework on 'RESTAPI' script execution
a4324f4 
- Fixed return of proper http error codes on script failure or
  internal failure to perform script execution. Introduced error
  codes:
    - 424: Script execution failed, either due to script failing to
      be executed, exiting with non-zero error code or output
      not being a valid JSON.
    - 500: Internal error while trying to execute script.
    - 200: Script was able to be executed properly, and result
      was properly parsed.
- Fixed dangling process left by 'return' in child process
  resulted from 'fork()' call.
- Fixed limitation of maximum return size of 48kb from executed
  script.
- Improved usage of 'read()' by using NONBLOCKING pipes.
- Fixed leaving child process alive after timeout expiration, now
  process is properly terminated.
- Improved general logging and returned JSONs in case of errors.
JavierJF added a commit that referenced this issue Mar 10, 2021
@JavierJF
Added regression test for #3223 major changes for 'RESTAPI'
ee86afe 
+ Added generic test for checking script execution through 'RESTAPI'.
+ Added multiple new scripts fails to be executed through the 'RESTAPI'.
@JavierJF JavierJF mentioned this issue Mar 10, 2021
Merged
renecannao added a commit that referenced this issue Mar 11, 2021
@renecannao
Merge pull request #3337 from sysown/v2.1.1-3223
461518a 
Closes #3223: Return proper errors codes on RESTAPI for script failures
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Closes #3223: Return proper errors codes on RESTAPI for script failures sysown/proxysql
2 participants
@akamac @renecannao