Simply retry strategy for minor errors #3380
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
|
OAGr
changed the title
| this.state = { | ||
| kind: "FAILED", | ||
| durationMs: this.calculateDuration(), | ||
| error, | ||
| errorType, | ||
| message, |
There was a problem hiding this comment.
There's some awkwardness here that makes me want to suggest exceptions as a failure mechanism.
I remember you don't like exceptions, so I'll explain my observations first.
- You log the error here (reasonable), but then sometimes you log it again, e.g. in
FixCodeUntilItRuns:
context.fail("MINOR", newCode.value);
context.log({
type: "codeRunError",
error: newCode.value,
});This will lead to duplicate error log entries.
Because you needed to type it as codeRunError... ok, one way around it is to pass the error to fail(), like, fail(errorType: ErrorType, message: Extract<LogEntry, { type: "error" | "codeRunError" }>), and then pass that. Fixable.
- But then there's also the awkwardness that the step code can call
fail()and then continue to do things. Which would be mostly ignored: the step is already failed, there should be nothing to do.
This also interferes with LLMStep.run(): you had to explicitly cast this.state as CodeState there, etc.
So, how about we just use throw MinorStepError(message) as the main way to fail? Then run() becomes more natural, if we catch an error then we minor-fail and either log the message or log String(error) if it's not a minor error. And we complete() otherwise.
One reason why I like this is because we already support exceptions; steps can throw, and we don't ask step implementations to carefully try/catch their code. So they might as well throw more meaningful errors.
In other words, fail("MINOR", "message") function is unnecessary, it's identical to throw new Error("message").
There was a problem hiding this comment.
Good idea, will experiment here.
This closes: #3375
Also, this does minor visual updates