Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workaround for #4555: copy the requestBuilder before mutation to avoid side effects #4556

Closed

Conversation

owengo
Copy link

@owengo owengo commented Oct 26, 2024

Pull Request Template

⚠️ Before Submitting a PR, Please Review:

  • Please ensure that you have thoroughly read and understood the Contributing Docs before submitting your Pull Request.

⚠️ Documentation Updates Notice:

  • Kindly note that documentation updates are managed in this repository: librechat.ai

Summary

Please provide a brief summary of your changes and the related issue. Include any motivation and context that is relevant to your changes. If there are any dependencies necessary for your changes, please list them here.

Change Type

Please delete any irrelevant options.

  • [ x] Bug fix (non-breaking change which fixes an issue)

Testing

I used the same repro procedure described in #4555 with the math assistant api.
When a custom auth is configured the assistant can successfully call the same action in parallel many times and have correct results for each of them.
Without this workaround all the results are the same.

Test Configuration:

Checklist

Please delete any irrelevant options.

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • [x ] My changes do not introduce new warnings

The commit is a workaround, a better solution would probably mean to separate the requestBuilder / ActionRequest from the parameters and headers used at execution time, or to no more share the requestBuilders between all the calls.
As it is it has a minimal impact on the code and fixes the issue with assistants.

@danny-avila
Copy link
Owner

Thanks for reporting this, addressing here: #4566

a better solution would probably mean to separate the requestBuilder / ActionRequest from the parameters and headers used at execution time, or to no more share the requestBuilders between all the calls.

I agree, so I took the time to test for the expected immutability/isolation between requests. Let me know if my fix addresses the issue.

@owengo owengo deleted the workarkound_side_effect_action_4555 branch October 28, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants