5098 branch 24 throw checked exception to remove todos

[Matilda-Clerke]

PR description

Final PR in the epic series of PRs. Throw checked exception at low level, and catch to remove TODOs. Also catch in a few places missed in previous reviews.

Fixed Issue(s)

#5098

[macfarla]

would it be nicer to modify getRequiredParameter() to throw the ex

hash = requestContext.getRequiredParameterOrElseThrow(0, Hash.class, "Invalid block hash parameter (index 0)", RpcErrorType.INVALID_BLOCK_HASH_PARAMS)

but there's a couple of spots where we do something different

[Matilda-Clerke]

That would be another way to get the details we want into the exception, but in general, I'd prefer to keep low level code as simple as possible, and let higher level code handle the specifics in the appropriate context.

As a middle ground, I could implement a set of methods in JsonRpcRequestContext like
public Hash getRequiredHashParameter(int index) which would clean up the higher level code, and still add detail to exceptions in the appropriate context.

[macfarla]

it would simplify slightly - I have no strong opinion on that - will leave up to you

[Matilda-Clerke]

I think given the length of time it would take, it's probably not worth doing right now

[jframe]

nice suggestion, it would make the code more readable removing a lot of try/catch blocks. but agree that it's probably not worth doing right now

[macfarla]

should this suppression be removed now?

[Matilda-Clerke]

This was unrelated to the changes I made. It seems we're retrieving an Object from the request context and performing an unchecked cast to a parameterized type. i.e.

final T param;
final Object rawParam = params[index];
if (paramClass.isAssignableFrom(rawParam.getClass())) {
  // If we're dealing with a simple type, just cast the value
  param = (T) rawParam;
} 

Looks like even though we're checking the type of T is appropriate for the object, the compiler (or intelliJ) is still complaining about the unchecked cast. We can get around this by using paramClass.cast(rawParam), which should have the same effect.

[jframe]

nice suggestion, it would make the code more readable removing a lot of try/catch blocks. but agree that it's probably not worth doing right now