core[patch]: Passing the input object to the Retry Attempt Handler.

[mauriciocirelli]

Hi,

This is related to the following discussion: How to capture and use the exception error on a Runnable With Retry Chain?.

Passing the input object to the retry attempt handler allows the developer to change it with the error information, making the chain capable of fixing the error in the next loop.

Retrying without fixing the error is very limited.. sometimes retrying the same calls would fix the issue, but usually, it doesnt.

Something like this would do the job:

RunnablePassthrough.assign({ 
  results: RunnableSequence.from([
	RunnablePassthrough.assign({ 
	  question: RunnableBranch.from([
		[
		  (input: any) => (input.error),
		  (input: any) => fix_errors(input.question, input.error),
		],
		(input: any) => input.question,
	  ]),
	}),
	RunnablePassthrough.assign({
	  // Clears the error
	  error: () => null,
	}),
	(input: any) => run_may_throw(input.question),
  ])
  .withRetry({
	onFailedAttempt: (error: any, input: any) => {
	  input.error = error.message;
	},
	stopAfterAttempt: 3,
  }),
})

The only thing that I would like to improve here is the async _batch<ReturnExceptions extends boolean = false>( inputs: RunInput[], configs?: RunnableConfig[], runManagers?: (CallbackManagerForChainRun | undefined)[], batchOptions?: RunnableBatchOptions) call (line 1375):

{
  onFailedAttempt: (error: any) => this.onFailedAttempt(error, inputs),
  retries: Math.max(this.maxAttemptNumber - 1, 0),
  randomize: true,
}

Is there a way to capture just the input that caused the exception? I am working around it passing the whole inputs array, but I think it would be nicer to just pass the faulty one.

I hope this interests the community.

Thank you.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
langchainjs-api-refs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 22, 2024 11:39pm
langchainjs-docs	✅ Ready (Inspect)	Visit Preview		Apr 22, 2024 11:39pm

[mauriciocirelli]

Hi,

Any thoughts on this?

[mauriciocirelli]

Hi, @jacoblee93

Any chance to have a look into this so we can get it merged?

Thank you!

[jacoblee93]

Sorry for the delay! Looking now!

[jacoblee93]

Hey @mauriciocirelli, does it make sense to pass the entire input back for batch? Feels like it'd make more sense to just pass back the first failure?

[mauriciocirelli]

Hey @mauriciocirelli, does it make sense to pass the entire input back for batch? Feels like it'd make more sense to just pass back the first failure?

Not at all.
That's where I got a bit lost on the code... I totally agree that we should pass just the input that caused the exception. It would also make the callback handler more standardized... But I could not find a way to do it. Would you mind pointing me how to get the input that caused the exception?

[jacoblee93]

What about just setting it on the thrown error:

          let firstException;
          for (let i = 0; i < results.length; i += 1) {
            const result = results[i];
            const resultMapIndex = remainingIndexes[i];
            // eslint-disable-next-line no-instanceof/no-instanceof
            if (result instanceof Error) {
              if (firstException === undefined) {
                firstException = result;
                (firstException as any).input = foo
              }
            }
            resultsMap[resultMapIndex.toString()] = result;
          }
          if (firstException) {
            throw firstException;
          }

[mauriciocirelli]

What about just setting it on the thrown error:

          let firstException;
          for (let i = 0; i < results.length; i += 1) {
            const result = results[i];
            const resultMapIndex = remainingIndexes[i];
            // eslint-disable-next-line no-instanceof/no-instanceof
            if (result instanceof Error) {
              if (firstException === undefined) {
                firstException = result;
                (firstException as any).input = foo
              }
            }
            resultsMap[resultMapIndex.toString()] = result;
          }
          if (firstException) {
            throw firstException;
          }

Sounds good.

Have just pushed the changes.

[jacoblee93]

LGTM just fix test case

[mauriciocirelli]

LGTM just fix test case

My bad. Looking into it.

[jacoblee93]

And run yarn format too please!

[mauriciocirelli]

And run yarn format too please!

Yeah. I have seen that workflow failing too.
Just pushed.

[jacoblee93]

Thank you!

[mauriciocirelli]

Thank you!

Im happy to contribute. Like this kind of PR, that helps making (mine and others) code cleaner.
Thank you for your time looking into it.

[jacoblee93]

Getting:

1371:36  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

Can fix myself in a bit

[mauriciocirelli]

Getting:

1371:36  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

Can fix myself in a bit

I am waiting for everything to finish so I can push it... do not worry.

It is good for me to learn the workflows and how to fix them.

[jacoblee93]

Thank you!