fix(internal-plugin-device): improve excessive registrations handling#4823
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44105d9fdc
ℹ️ 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). |
chrisadubois
left a comment
chrisadubois
left a comment
There was a problem hiding this comment.
small comments, besides the ones the AI provided.
robstax
left a comment
robstax
left a comment
There was a problem hiding this comment.
1. previously there was a device check of at least 2 devices. if there's only 1-2 devices, this will try to delete one of them still. could something unpredictable happen there? like delete itself? or was the previous if (sortedDevices.length > 2) there for a reason?
2. also the _getDevicesOfCurrentType() if that fails for some reason, i think the user will just get stuck and forced to reload
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cab16223c
ℹ️ 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".
…ead/improve_excessive_registrations_handling
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5af6f35f64
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7508ee56d6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e799e6e05
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be9b96b081
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fef01d713b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 272c78deeb
ℹ️ 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".
| if (sortedDevices.length <= MIN_DEVICES_FOR_CLEANUP) { | ||
| this.logger.info( | ||
| `device: only ${sortedDevices.length} devices found (minimum ${MIN_DEVICES_FOR_CLEANUP}), skipping cleanup` | ||
| ); | ||
|
|
||
| return Promise.resolve(); |
There was a problem hiding this comment.
When registration fails with User has excessive device registrations, register() relies on deleteDevices() to free capacity before the single retry. This new guard returns early for <= 5 same-type devices, so users with mixed device fleets (for example, 101 total devices but only 3 WEB entries) perform no cleanup and the retry is very likely to fail again. This is a regression from the previous behavior (>2 threshold) that could block registration in production for accounts near the global limit.
Useful? React with 👍 / 👎.
Thank you for your contribution! |
3 participants
COMPLETES #< INSERT LINK TO ISSUE >
This pull request addresses
Registration would occasionally fail due to excessive registrations despite existing mitigation
by making the following changes
The code previously returned when the first delete succeeded due to Promise.race. There appears to be a server side race whereby the next registration would happen and fail due to the back end returning but not updating its count.
Attempt to mitigate this by confirming the delete with a fetch afterwards. This obviously adds some time to the process but it is better than it occasionally failing and throwing during device registration.
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.