-
Notifications
You must be signed in to change notification settings - Fork 396
fix(internal-plugin-device): improve excessive registrations handling #4823
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
Changes from all commits
44105d9
9c9cd02
d35603b
3cab162
5af6f35
7508ee5
5e799e6
be9b96b
fef01d7
272c78d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,13 @@ import {orderBy} from 'lodash'; | |
| import uuid from 'uuid'; | ||
|
|
||
| import METRICS from './metrics'; | ||
| import {FEATURE_COLLECTION_NAMES, DEVICE_EVENT_REGISTRATION_SUCCESS} from './constants'; | ||
| import { | ||
| FEATURE_COLLECTION_NAMES, | ||
| DEVICE_EVENT_REGISTRATION_SUCCESS, | ||
| MIN_DEVICES_FOR_CLEANUP, | ||
| MAX_DELETION_CONFIRMATION_ATTEMPTS, | ||
| DELETION_CONFIRMATION_DELAY_MS, | ||
| } from './constants'; | ||
| import FeaturesModel from './features/features-model'; | ||
| import IpNetworkDetector from './ipNetworkDetector'; | ||
| import {CatalogDetails} from './types'; | ||
|
|
@@ -454,46 +460,117 @@ const Device = WebexPlugin.extend({ | |
| }); | ||
| }, | ||
| /** | ||
| * Fetches the web devices and deletes the third of them which are not recent devices in use | ||
| * @returns {Promise<void, Error>} | ||
| * Fetches devices matching the current device type. | ||
| * @returns {Promise<Array>} filtered device list | ||
| */ | ||
| deleteDevices() { | ||
| // Fetch devices with a GET request | ||
| _getDevicesOfCurrentType() { | ||
| const {deviceType} = this._getBody(); | ||
|
|
||
| return this.request({ | ||
| method: 'GET', | ||
| service: 'wdm', | ||
| resource: 'devices', | ||
| }) | ||
| .then((response) => { | ||
| const {devices} = response.body; | ||
| }).then((response) => response.body.devices.filter((item) => item.deviceType === deviceType)); | ||
| }, | ||
|
|
||
| const {deviceType} = this._getBody(); | ||
| /** | ||
| * Waits until the server-side device count drops to or below targetCount, | ||
| * polling up to maxAttempts times with a delay between each check. | ||
| * @param {number} targetCount - resolve when device count drops to this value or below | ||
| * @param {number} [attempt=0] | ||
| * @returns {Promise<void>} | ||
| */ | ||
| _waitForDeviceCountBelowLimit(targetCount, attempt = 0) { | ||
| if (attempt >= MAX_DELETION_CONFIRMATION_ATTEMPTS) { | ||
| this.logger.warn('device: max confirmation attempts reached, proceeding anyway'); | ||
|
|
||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| return new Promise((resolve) => setTimeout(resolve, DELETION_CONFIRMATION_DELAY_MS)) | ||
| .then(() => this._getDevicesOfCurrentType()) | ||
| .then((devices) => { | ||
| this.logger.info( | ||
| `device: confirmation check ${attempt + 1}/${MAX_DELETION_CONFIRMATION_ATTEMPTS}, ` + | ||
| `${devices.length} devices remaining (target: ≤ ${targetCount})` | ||
| ); | ||
|
|
||
| if (devices.length <= targetCount) { | ||
| this.logger.info('device: device count is now safely below limit'); | ||
|
|
||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| return this._waitForDeviceCountBelowLimit(targetCount, attempt + 1); | ||
| }) | ||
| .catch((error) => { | ||
| this.logger.warn( | ||
| `device: confirmation check ${attempt + 1} failed, proceeding anyway:`, | ||
| error | ||
| ); | ||
|
|
||
| return Promise.resolve(); | ||
| }); | ||
| }, | ||
|
|
||
| // Filter devices of type deviceType | ||
| const webDevices = devices.filter((item) => item.deviceType === deviceType); | ||
| /** | ||
| * Fetches the web devices and deletes the oldest third, then waits | ||
| * for the server to confirm the count is below the limit. | ||
| * @returns {Promise<void>} | ||
| */ | ||
| deleteDevices() { | ||
| let targetCount; | ||
|
|
||
| return this._getDevicesOfCurrentType() | ||
| .then((webDevices) => { | ||
| const sortedDevices = orderBy(webDevices, [(item) => new Date(item.modificationTime)]); | ||
|
|
||
| // If there are more than two devices, delete the last third | ||
| if (sortedDevices.length > 2) { | ||
| const totalItems = sortedDevices.length; | ||
| const countToDelete = Math.ceil(totalItems / 3); | ||
| const urlsToDelete = sortedDevices.slice(0, countToDelete).map((item) => item.url); | ||
|
|
||
| return Promise.race( | ||
| urlsToDelete.map((url) => { | ||
| return this.request({ | ||
| uri: url, | ||
| method: 'DELETE', | ||
| }); | ||
| }) | ||
| 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(); | ||
|
Comment on lines
+528
to
+533
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When registration fails with Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| return Promise.resolve(); | ||
| const devicesToDelete = sortedDevices.slice(0, Math.ceil(sortedDevices.length / 3)); | ||
| targetCount = sortedDevices.length - Math.min(5, devicesToDelete.length); | ||
|
Coread marked this conversation as resolved.
Coread marked this conversation as resolved.
|
||
|
|
||
| this.logger.info( | ||
| `device: deleting ${devicesToDelete.length} of ${webDevices.length} devices` | ||
| ); | ||
|
|
||
| return Promise.all( | ||
| devicesToDelete.map((device) => | ||
| this.request({uri: device.url, method: 'DELETE'}) | ||
| .then(() => ({status: 'fulfilled'})) | ||
| .catch((reason) => ({status: 'rejected', reason})) | ||
| ) | ||
| ).then((results) => { | ||
| const failed = results.filter((r) => r.status === 'rejected'); | ||
|
|
||
| if (failed.length > 0) { | ||
| this.logger.warn( | ||
| `device: ${failed.length} of ${devicesToDelete.length} deletions failed (best-effort, continuing)` | ||
| ); | ||
| } | ||
| this.logger.info( | ||
| `device: deleted ${devicesToDelete.length - failed.length} of ${ | ||
| devicesToDelete.length | ||
| } devices` | ||
| ); | ||
| }); | ||
| }) | ||
| .then(() => | ||
| targetCount !== undefined | ||
| ? this._waitForDeviceCountBelowLimit(targetCount, 0) | ||
| : Promise.resolve() | ||
| ) | ||
| .then(() => { | ||
| this.logger.info('device: device count confirmed below limit, cleanup successful'); | ||
| }) | ||
| .catch((error) => { | ||
| this.logger.error('Failed to retrieve devices:', error); | ||
| this.logger.error('device: failed to delete devices:', error); | ||
|
|
||
| return Promise.reject(error); | ||
| }); | ||
|
|
@@ -519,7 +596,11 @@ const Device = WebexPlugin.extend({ | |
|
|
||
| return this._registerInternal(deviceRegistrationOptions).catch((error) => { | ||
| if (error?.body?.message === 'User has excessive device registrations') { | ||
| this.logger.info('device: excessive device registrations detected, initiating cleanup'); | ||
|
|
||
| return this.deleteDevices().then(() => { | ||
| this.logger.info('device: device cleanup complete, retrying registration'); | ||
|
|
||
| return this._registerInternal(deviceRegistrationOptions); | ||
| }); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.