fix(internal-plugin-support,plugin-logger): when log upload fails, upload the failed logs in the next attempt#4996
Conversation
…in the next attempt
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1120b74b3e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c20b0a8a29
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex rereview this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa502a18c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return this.webex | ||
| .upload(uploadOptions) | ||
| .then((res) => { | ||
| this.webex.logger.updateLastSubmittedIndex(); |
There was a problem hiding this comment.
Tie the checkpoint to the upload that just succeeded
When two incremental log uploads overlap, this marks the shared logger checkpoint using the current nextIndex, not the index range that this particular upload actually sent. The meetings code can fire uploads without awaiting earlier ones (for example setLogUploadTimer() calls this.uploadLogs() and immediately schedules the next timer, and other meeting events call uploadLogs() too), so if upload A formats logs 0..N, upload B then formats newer logs and advances nextIndex, and only A succeeds, this line records B's unsent range as successfully submitted; a later B failure will reset to that advanced lastSubmitted and skip retrying B's logs. Capture the indexes returned/advanced by formatLogs({diff: true}) per upload and only checkpoint those after that same upload succeeds.
Useful? React with 👍 / 👎.
This pull request addresses
If uploading the incremental logs hits a 429, we lose the logs being submitted to the backend in that attempt.
by making the following changes
Reset the nextIndex to the last submitted index when the upload of the incremental logs fails.
This logic is behind a config value (
retryFailedLogUploadsAtNextInterval) as to not breaking existing behaviour.Change Type
The following scenarios were tested
Tested by blocking the requests in the Devtools and checking the internal property values followed by unblocking the request and checking the values again
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.