📸 fix: Snapshot Options to Prevent Mid-Await Client Disposal Crash#12398
Merged
Conversation
…disposal Add null guard in saveMessageToDatabase to handle the case where disposeClient nullifies this.options while a userMessagePromise is still pending from a prior async save operation.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent a Node process crash caused by a race condition where disposeClient() nulls client.options while an in-flight saveMessageToDatabase() async operation is still running (triggering TypeError: Cannot read properties of null (reading 'endpoint')).
Changes:
- Add an early null-guard in
BaseClient.saveMessageToDatabase()to skip saving when the client has already been disposed. - Emit a warning log when a post-disposal save is attempted.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+730
to
+733
| if (!this.options) { | ||
| logger.warn('[BaseClient] saveMessageToDatabase called after client disposal, skipping save'); | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
The null guard only protects the beginning of the method. saveMessageToDatabase still dereferences this.options.endpoint/this.options.endpointType after an await (e.g., when building fieldsToKeep), so disposeClient() can still set this.options = null mid-flight and trigger the same TypeError. Fix by snapshotting the needed values from this.options into locals before the first await and avoid reading this.options after awaits, or re-check this.options after await db.saveMessage(...) and return early (e.g., skip convo save) if disposed.
Comment on lines
+730
to
+733
| if (!this.options) { | ||
| logger.warn('[BaseClient] saveMessageToDatabase called after client disposal, skipping save'); | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
Add a regression test that simulates disposal during the await inside saveMessageToDatabase (e.g., delay db.saveMessage/db.saveConvo, set client.options = null before the promise resolves) and asserts the method resolves without throwing. This will cover the race described in the PR and prevent future reintroductions.
The original guard at function entry was insufficient — this.options is always valid at entry. The crash occurs after the first await (db.saveMessage) when disposeClient nullifies client.options while the promise is suspended. Fix: capture this.options into a local const before any await. The local reference is immune to client.options = null set by disposeClient. Also add .catch on userMessagePromise in sendMessage to prevent unhandled rejections when checkBalance throws before the promise is awaited, and add two regression tests.
danny-avila
changed the title
berry-13
pushed a commit
that referenced
this pull request
Mar 26, 2026
…12398) * 🐛 fix: Prevent crash when token balance exhaustion races with client disposal Add null guard in saveMessageToDatabase to handle the case where disposeClient nullifies this.options while a userMessagePromise is still pending from a prior async save operation. * 🐛 fix: Snapshot this.options to prevent mid-await disposal crash The original guard at function entry was insufficient — this.options is always valid at entry. The crash occurs after the first await (db.saveMessage) when disposeClient nullifies client.options while the promise is suspended. Fix: capture this.options into a local const before any await. The local reference is immune to client.options = null set by disposeClient. Also add .catch on userMessagePromise in sendMessage to prevent unhandled rejections when checkBalance throws before the promise is awaited, and add two regression tests.
yidianyiko
pushed a commit
to yidianyiko/LibreChat
that referenced
this pull request
Apr 13, 2026
…anny-avila#12398) * 🐛 fix: Prevent crash when token balance exhaustion races with client disposal Add null guard in saveMessageToDatabase to handle the case where disposeClient nullifies this.options while a userMessagePromise is still pending from a prior async save operation. * 🐛 fix: Snapshot this.options to prevent mid-await disposal crash The original guard at function entry was insufficient — this.options is always valid at entry. The crash occurs after the first await (db.saveMessage) when disposeClient nullifies client.options while the promise is suspended. Fix: capture this.options into a local const before any await. The local reference is immune to client.options = null set by disposeClient. Also add .catch on userMessagePromise in sendMessage to prevent unhandled rejections when checkBalance throws before the promise is awaited, and add two regression tests.
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
…anny-avila#12398) * 🐛 fix: Prevent crash when token balance exhaustion races with client disposal Add null guard in saveMessageToDatabase to handle the case where disposeClient nullifies this.options while a userMessagePromise is still pending from a prior async save operation. * 🐛 fix: Snapshot this.options to prevent mid-await disposal crash The original guard at function entry was insufficient — this.options is always valid at entry. The crash occurs after the first await (db.saveMessage) when disposeClient nullifies client.options while the promise is suspended. Fix: capture this.options into a local const before any await. The local reference is immune to client.options = null set by disposeClient. Also add .catch on userMessagePromise in sendMessage to prevent unhandled rejections when checkBalance throws before the promise is awaited, and add two regression tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TypeError: Cannot read properties of null (reading 'endpoint')that crashes the Node process when a user exhausts their token balancesendMessage()fires an asyncuserMessagePromiseviasaveMessageToDatabase()(without awaiting it), thencheckBalance()throws, and the error handler callsdisposeClient()which setsclient.options = null— while the DB save promise is still suspended at anawaitthis.optionsinto a localconst optionsbefore anyawaitinsaveMessageToDatabase(), then use the local reference throughout.disposeClientsetsclient.options = null(nulling the property on the client), but the local variable retains its reference to the original object — immune to external mutation.catch()onuserMessagePromiseinsendMessage()to prevent unhandled rejections if the promise rejects aftercheckBalancethrows (since no one awaits it on the error path)Test plan
saveMessageToDatabasereturns{}early whenthis.optionsis null at entrysaveMessageToDatabasecompletes without TypeError whenthis.optionsis nullified mid-await (snapshot protects all post-await accesses)