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

getHeader: 204 (no content) response if no header is available #34

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

metachris
Copy link
Contributor

@metachris metachris commented Jun 24, 2022

See also #32

If no header is available, status code 204 and no content can be returned. 204 seems appropriate: https://www.webfx.com/web-development/glossary/http-status-codes/what-is-a-204-status-code/

Would be returned from the relay if it has no header naturally, or during the period after the merge where we manually disable headers.

@metachris metachris changed the title getHeader: 204 response if no header is available getHeader: 204 (no content) response if no header is available Jun 24, 2022
@ralexstokes
Copy link
Member

ralexstokes commented Jun 27, 2022

I think 204 fits for this use case

although I would also think it is fine to just return an error, rather than add more feature surface into the builder specs

wdyt?

edit: thought about this a bit more and you could easily have a relay w/o a header (e.g. builders didn't supply one for some reason) for a given slot and then I think a 204 would make sense here

@metachris
Copy link
Contributor Author

Also if the relay is not shipping blocks for a few epochs after the merge, I'd rather not return an error on every block (which will pollute the mev-boost and BN logs with errors as well). A clean no-content response would be much preferable imo.

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

This makes a lot of sense to me.

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