[lojack5]

Make a more generic exception type (engine::runtime::RequestValidationError) to throw when the client is able to correct the problem by changing request parameters, converted to an HTTP 400 response.

Follow ups can go through each model to turn existing thrown exceptions into throwing a RequestValidationError where appropriate.

[lojack5]

@0xShug0 here's the basic change to make it more obvious what to throw when model's just receive invalid request parameters. For testing, I started digging into the tests code to get an understanding, but ran into some questions so figured I'd ask before I start doing anything elaborate.

What sort of test are you looking for here? A full, load model -> send bad request -> test that response is 400 with proper message? + test existing errors retain their old behavior (CapacityError -> 400, std::exception w/ CORS -> 500)

I noticed that test_server_parallel_lifecycle has a whole fixture system setup so that it can test stuff like this (if all you want tested is what the server does with specific exception classes) without having to load a model. But, I didn't find any similar fixture system anywhere for the non-parallel runtime.cpp.

I could potentially:

• Put a test in test_server_parallel_lifecycle that test parallel_runtime.cpp's request triggered exception -> HTTP response routing.
• Copy the idea (but much simpler, because it wouldn't need to handle synchronization and such) for a reusable text fixture against runtime.cpp, then make a test file that tests the exception routing in that path.

If you want a full model load + test, I could pretty simply do it with Echo-TTS for this PR (EchoTtsSession::parse_sampler_options already throws 3 runtime_exceptions that look like they could become a RequestValidationError), outside of the additional request errors I plan to add in the joint-cfg PR.

[lojack5]

Although... looks like I messed something up, build errors. Let me fix that first.
Edit: done

[0xShug0]

@lojack5 A lightweight exception-mapping test is enough for this PR; no real model load is needed. Please cover both server handlers. Reuse the existing fixture where practical, but don’t introduce a larger testing framework just for this.

[lojack5]

@0xShug0 Took a bit longer to work on this, because I wanted to get the non-parallel ServerState exception handling working without loading a model (plus I was busy).

I ended up needing a similar fixture system for the non-parallel version, I've pushed (though it's not ready, need to rebase to incorporate recent work / resolve conflicts) and would like a look to see if you're ok with what I've done so far:

• Adding infrastructure for testing the ServerState handle_request
• Needed to test similarly with the ParallelServerState: pulled out the existing fixtures and pulled it into a common header for it, added the few small features needed for testing the exception handlers -> HTTP error codes

I'm not, exactly happy with it. I debated using two headers, one for non-parallel, one for parallel. That is an option, which would you prefer? Or, would you prefer I not touch the parallel fixtures directly and just shove the needed tests into test_server_parallel_lifetime test file?

I can also probably deduplicate a bunch of the work the two versions do - though I'll probably need to template on ServerState vs ParallelServerState (and there is one minor internal difference I ran into, though I think I can work around that). Fair warning on this one - it's been ages since I've touched templated c++ code so not sure how long that would take.

Anyway, just wanted you to take a look and see if you like this route or if you want me to do something different (ex: only add the non-parallel fixture, then shove the testing for the parallel version into the parallel lifetime tests) before I start working on conflict resolution - I see recent work already added a InvalidRequestError and similar tests for fm2_audio, so I'll need to resolve that. But I'd prefer to get your feedback before I start on more work that might be going down the wrong path.

[0xShug0]

@lojack5 Thanks for following up. A lot has happened over the past week and this PR has now been superseded by #838 and #844, which added InvalidRequestError, HTTP 400 handling, and coverage for both server paths. The additional exception hierarchy and fixture infrastructure are no longer needed. I'd suggest closing this PR and focusing on the Echo-TTS Joint-CFG work in #773 instead.

[lojack5]

Gotcha, fair enough. Easy enough for me, plus I got a bit more familiar with the internals by doing this.