Skip to content

return InvalidArgument for non-103 1xx status codes#557

Merged
joeshaw merged 1 commit into
mainfrom
joeshaw/push-krxyopvrrvnt
Nov 19, 2025
Merged

return InvalidArgument for non-103 1xx status codes#557
joeshaw merged 1 commit into
mainfrom
joeshaw/push-krxyopvrrvnt

Conversation

@joeshaw
Copy link
Copy Markdown
Member

@joeshaw joeshaw commented Nov 18, 2025

Add an integration test for this. With the Rust SDK this results in a
panic and a 500 response, which we check for in the test.

Add an integration test for this.  With the Rust SDK this results in a
panic and a 500 response, which we check for in the test.
@joeshaw joeshaw requested review from bahamat and ulyssa and removed request for ulyssa November 19, 2025 13:46
@bahamat
Copy link
Copy Markdown
Member

bahamat commented Nov 19, 2025

My philosophy when I implemented this was that 1xx other than 103 would continue to exhibit whatever behavior it had before I made changes.

I'm not saying this change shouldn't be made, necessarily. But that was why I had decided not to add additional handling.

@joeshaw
Copy link
Copy Markdown
Member Author

joeshaw commented Nov 19, 2025

That makes sense! My motivation for opening this is to make the behavior consistent with Compute. Otherwise it makes testing harder, especially in the SDKs. For example, fastly/compute-sdk-go#218 and specifically https://github.com/fastly/compute-sdk-go/blob/54e5d57017e61504bc7500725f68e7c0428dfe8d/end_to_end_tests/loopback/main_test.go#L66-L87

@ulyssa
Copy link
Copy Markdown
Member

ulyssa commented Nov 19, 2025

Thank you for implementing this! Yes, mirroring Compute's behaviour here is the right thing to do. If we ever support other 1xx codes beyond 103, then we can update this to allow those through in the same way.

@joeshaw joeshaw merged commit da6b81f into main Nov 19, 2025
13 checks passed
@joeshaw joeshaw deleted the joeshaw/push-krxyopvrrvnt branch November 19, 2025 22:36
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