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

fix: Better restart feedback & attempt to fix CORS #455

Merged
merged 2 commits into from
Nov 9, 2024

Conversation

mrv777
Copy link
Contributor

@mrv777 mrv777 commented Nov 5, 2024

Description

  • If swarm restart fails, show an error instead of Success
  • Attempt to fix CORS error that prevents restart from swarm

Should address #154

// Set CORS headers
if (set_cors_headers(req) != ESP_OK) {
httpd_resp_send_500(req);
return ESP_FAIL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that I believe we would ever fail to set cors, but I believe you always need to return ESP_OK when intending to send a response to the client e.g. HTTP 500.
See #424

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so should I change that. Sorry, I'm not very familiar with the ESP code

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep. I think this is largely undocumented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I copied and pasted that from the other functions, so I updated them all to return ESP_OK. Let me know if that's an issue, but figured they should all do that then

@WantClue WantClue added bug Something isn't working enhancement New feature or request accepted This issue will be worked on labels Nov 9, 2024
@WantClue WantClue merged commit 21a8f64 into skot:master Nov 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This issue will be worked on bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants