-
Notifications
You must be signed in to change notification settings - Fork 1.2k
PVF worker: refactor worker/job errors #2195
Description
As part of #1685 I did a big refactor of errors, but they need some more work. I left it out of that PR because it was already turning into a monster.
As I was refactoring errors in #1685 I noticed some security issues. Some errors from the execute job were being treated as internal, even though malicious code can send back arbitrary errors! That's why it's important that the error handling here is robust and accurate.
1. Refactor execute WorkerResponse (#2604)
We should refactor WorkerResponse for more idiomatic error handling:
type WorkerResult = Result<WorkerResponse, WorkerError>.
That is, move the Ok variant into a WorkerResponse type and the rest of the variants into
WorkerError. See e.g. JobResponse and JobError.
2. Move more of the execute WorkerIntfErrors to InternalValidationError (#2604)
Now that the worker and job are separate processes, some worker-specific errors can potentially be treated as internal now.
3. Have JobResponse/JobError types for prepare like we did for execute
Right now preparation returns a PrepareResult, which gets turned into an Outcome, which gets turned into a PrepareError again. Note that in prepration itself, many of the variants of PrepareError are never constructed. We should have a JobError type that limits the possible errors of a job, like we did for execution jobs.
4. Don't allow prepare jobs to raise non-deterministic errors
It's an existing issue, and not so critical for preparation as for execution, so I left it out of the PR. It's not critical because we treat all preparation errors as internal because we assume that real errors were caught and filtered during prechecking.
But still, we should get the story right. It could make prechecking more secure. So, preparation jobs should not trigger internal preparation errors. See NonDeterministicPrepareError.
Related
Follow-up of #1685
Metadata
Metadata
Assignees
Labels
Type
Projects
Status