fix: remove ErrorEntity and throw on connector error instead

[vcrfxia]

Description

This PR is stacked on #8983. Only the commits starting with chore: remove ErrorEntity and throw on connector error instead need to be reviewed.

Currently, when connector requests encounter errors they return a 200 response code with an ErrorEntity object in the response. This is problematic because:

• the 200 response code is confusing, given that the request did not succeed
• in the case when multiple ksql statements are issued as a single request, the 200 response code from a failed connector statement causes subsequent statements to be executed even though they shouldn't be.

This PR updates the connect executors to return an error response (resulting in a non-200 response code) when requests fail.

Testing done

Unit + integration + manual.

Reviewer checklist

• Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• Ensure relevant issues are linked (description should include text like "Fixes #")

[vcrfxia]

I've removed this validation because it was a no-op -- validation only fails if an error is thrown, and no error was ever thrown from DescribeConnectorExecutor::validate since it returned a 200 response with an error entity on error, instead of throwing.

It doesn't make sense to validate this request because there's nothing to validate (validating it amounts to issuing the request to Connect, which is what execution does). The same logic applies to DropConnector and ListConnectors, both of which are already not being validated.

See #8999 for more on validating connector statements.

[pgaref]

Good catch, DescribeConnectorExecutor::execute never throws an exception indeed.

I guess we should do the same for LIST_VARIABLES and DESCRIBE_FUNCTION -- can do it in a follow-up

[pgaref]

nit: move errorMsg to a final var for consistency and readability -- applies to all KsqlRestExceptions

[vcrfxia]

Hm interesting. I think most places in the codebase inline the error message directly rather than having a separate line for final String errorMsg = ...; but I do see a few places where we opt for the latter and are inconsistent as a result. For relatively short error messages such as this one, I don't personally think it's harder to read when the error message is inlined but I don't feel too strongly. I'll make the change.

[pgaref]

Assuming client connection failed here thus KsqlException instead of KsqlRestException?

[vcrfxia]

Not quite -- KsqlException means it's a user error, which is necessarily the case if there's a validation error but not necessarily the case if other errors are encountered.

The final exception type returned from the engine is always KsqlRestException. KsqlException is automatically wrapped in KsqlRestException here:

ksql/ksqldb-rest-app/src/main/java/io/confluent/ksql/rest/server/resources/KsqlResource.java

Lines 328 to 341 in 7b3fb6b

	} catch (final KsqlRestException e) {
	LOG.info("Processed unsuccessfully: " + request + ", reason: ", e);
	throw e;
	} catch (final KsqlStatementException e) {
	LOG.info("Processed unsuccessfully: " + request + ", reason: ", e);
	return Errors.badStatement(e.getRawMessage(), e.getSqlStatement());
	} catch (final KsqlException e) {
	LOG.info("Processed unsuccessfully: " + request + ", reason: ", e);
	return errorHandler.generateResponse(e, Errors.badRequest(e));
	} catch (final Exception e) {
	LOG.info("Processed unsuccessfully: " + request + ", reason: ", e);
	return errorHandler.generateResponse(
	e, Errors.serverErrorForStatement(e, request.getKsql()));
	}

For the non-validation connector errors, we don't know whether it's a user error or not without inspecting the error itself, which is why this PR simply returns a KsqlRestException directly rather than trying to figure out what inner error to throw in those cases.

[Gerrrr]

This PR looks good!