Skip to content

Conversation

@nigoroll
Copy link
Member

@nigoroll nigoroll commented Nov 25, 2024

Re #4232

Adding earlier overflow checks is pretty easy if we use VCL failure, but so far we had centralized the overflow handling on the client side in the vtr_deliver_f, sending downstream a 500 error and not a 503.
So my main question is: If we want consistency here, can we settle on simply handling workspace overflows as VCL failures? Or do we really want the separate error code still? If we do, improving the overflow detection on the backend side might become a bit more involved, because right now the backend side error just turns into a 503 on the client side.

Note that this PR is really for discussing the design questions, it does not pass the test suite because of the changed error code.

@nigoroll
Copy link
Member Author

nigoroll commented Dec 2, 2024

To make progress here, I'd suggest the following:

  • turn all VCL and out-of-workspace related errors into 500 because this is really what they are. 503 makes specific reference to a temporary overload or scheduled maintenance, but if we run into a VCL failure or out-of-workspace, that is likely to persist for the next request.
  • differentiate other error conditions. I think we have also more tickets in this area...

@nigoroll
Copy link
Member Author

nigoroll commented Dec 9, 2024

bugwash: turn it all into 500, update PR, write doc

@nigoroll nigoroll self-assigned this Dec 9, 2024
@nigoroll nigoroll added the a=Implement Issue is ready for implementation label Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a=Implement Issue is ready for implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant