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

Update docs with return payload #469

Merged
merged 6 commits into from
Sep 5, 2024
Merged

Update docs with return payload #469

merged 6 commits into from
Sep 5, 2024

Conversation

iansuvak
Copy link
Contributor

@iansuvak iansuvak commented Sep 3, 2024

Why this should be merged

Returns error status codes and adds response documentation to the signature aggregator API

How this works

How this was tested

How is this documented

@michaelkaplan13
Copy link
Collaborator

We have previously gone with the standard of leaving HTTP status codes to any network level issues/infra, and always returning 200 with an error message for any application level errors. To stay consistent, I'd recommend we do that here as well to match other platform APIs

@iansuvak
Copy link
Contributor Author

iansuvak commented Sep 4, 2024

We have previously gone with the standard of leaving HTTP status codes to any network level issues/infra, and always returning 200 with an error message for any application level errors. To stay consistent, I'd recommend we do that here as well to match other platform APIs

Sure thing -- I'll remove references but I am curious about the reasoning here. In general I think that there is value in distinguishing between client caused failures and internal errors

@iansuvak iansuvak changed the title Return Error HTTP status codes + update docs Update docs with return payload Sep 4, 2024
@iansuvak iansuvak merged commit 576693d into main Sep 5, 2024
8 checks passed
@iansuvak iansuvak deleted the api-error-status branch September 5, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants