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: Return error and exception when endpoint app raises an exception #820

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

dleviminzi
Copy link
Collaborator

Resolve BE-2148

Once the warning message has been circulated and users have had time to update any handling logic we can merge this.

It is the exact same thing as #791, but we need to wait to release

Copy link
Contributor

@luke-lombardi luke-lombardi left a comment

Choose a reason for hiding this comment

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

LGTM - cc @mernit to communicate the change in behavior to users. Basically now if your endpoint throws an error you'll get a 500 instead of a 200 as you do now.

@luke-lombardi
Copy link
Contributor

LGTM - cc @mernit to communicate the change in behavior to users. Basically now if your endpoint throws an error you'll get a 500 instead of a 200 as you do now.

@mernit For more context, @dleviminzi's original message:

Previously, if the handler raised an exception it would be returned with status 200 and the task would be marked complete. With this change, the exception is returned with status 500 and the task is marked as errored.

@mernit
Copy link
Contributor

mernit commented Jan 5, 2025

@luke-lombardi we already communicated this in Slack, I told them we'd release it Thursday

@dleviminzi dleviminzi merged commit 3a47b58 into main Jan 10, 2025
3 checks passed
@dleviminzi dleviminzi deleted the dlm/endpoint-return-error-on-exception branch January 10, 2025 20:13
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.

3 participants