-
Notifications
You must be signed in to change notification settings - Fork 4
refactor: streamline device identifier handling and error management #230
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
fe75aaf
6cc7dee
ea560a2
e5973b4
4208aec
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 |
|---|---|---|
| @@ -1,35 +1,37 @@ | ||
| import { DependencyInjectionUserProvider } from './../../../apps/shared/dependency-injection/DependencyInjectionUserProvider'; | ||
| import { Device } from './../../../apps/main/device/service'; | ||
| import { createNewDevice } from './createNewDevice'; | ||
| import { BrowserWindow } from 'electron'; | ||
| import { broadcastToWindows } from '../../../apps/main/windows'; | ||
| import { logger } from '@internxt/drive-desktop-core/build/backend'; | ||
| import { getDeviceIdentifier } from './getDeviceIdentifier'; | ||
|
|
||
| export async function createAndSetupNewDevice(): Promise<Device | Error> { | ||
| const getDeviceIdentifierResult = getDeviceIdentifier(); | ||
| if (getDeviceIdentifierResult.isLeft()) { | ||
| return getDeviceIdentifierResult.getLeft(); | ||
| } | ||
| const deviceIdentifier = getDeviceIdentifierResult.getRight(); | ||
| export async function createAndSetupNewDevice() { | ||
| const { error, data: deviceIdentifier } = getDeviceIdentifier(); | ||
| if (error) return { error }; | ||
|
|
||
| const createNewDeviceEither = await createNewDevice(deviceIdentifier); | ||
| if (createNewDeviceEither.isRight()) { | ||
| const device = createNewDeviceEither.getRight(); | ||
| const user = DependencyInjectionUserProvider.get(); | ||
| user.backupsBucket = device.bucket; | ||
|
|
||
| const mainWindow = BrowserWindow.getAllWindows()[0]; | ||
| if (mainWindow) { | ||
| mainWindow.webContents.send('reinitialize-backups'); | ||
| } | ||
| broadcastToWindows('device-created', device); | ||
| logger.debug({ | ||
| if (createNewDeviceEither.isLeft()) { | ||
| logger.error({ | ||
| tag: 'BACKUPS', | ||
| msg: '[DEVICE] Created new device', | ||
| deviceUUID: device.uuid, | ||
| msg: '[DEVICE] Error creating new device', | ||
| error: createNewDeviceEither.getLeft(), | ||
| }); | ||
| return device; | ||
| return { error: createNewDeviceEither.getLeft() }; | ||
| } | ||
|
|
||
| const device = createNewDeviceEither.getRight(); | ||
| const user = DependencyInjectionUserProvider.get(); | ||
| user.backupsBucket = device.bucket; | ||
|
|
||
| const mainWindow = BrowserWindow.getAllWindows()[0]; | ||
| if (mainWindow) { | ||
| mainWindow.webContents.send('reinitialize-backups'); | ||
| } | ||
| return createNewDeviceEither.getLeft(); | ||
| broadcastToWindows('device-created', device); | ||
| logger.debug({ | ||
| tag: 'BACKUPS', | ||
| msg: '[DEVICE] Created new device', | ||
| deviceUUID: device.uuid, | ||
| }); | ||
| return { data: device }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,86 +1,59 @@ | ||
| import { driveServerModule } from './../../../infra/drive-server/drive-server.module'; | ||
| import { Either, left, right } from '../../../context/shared/domain/Either'; | ||
| import { Device } from '../../../apps/main/device/service'; | ||
| import { logger } from '@internxt/drive-desktop-core/build/backend'; | ||
| import { BackupError } from '../../../infra/drive-server/services/backup/backup.error'; | ||
| import { addUnknownDeviceIssue } from './addUnknownDeviceIssue'; | ||
| import { DeviceIdentifierDTO } from './device.types'; | ||
|
|
||
| export type FetchDeviceProps = { deviceIdentifier: DeviceIdentifierDTO } | { uuid: string } | { legacyId: string }; | ||
|
|
||
| async function getDeviceByProps(props: FetchDeviceProps): Promise<Either<Error, Device | null>> { | ||
| async function getDeviceByProps(props: FetchDeviceProps) { | ||
|
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. [Non blocking] Just to give some information. We found that maybe this is no longer necessary since this is just a backup to recover to the device identifier of the user in case the app is uninstalled. However, the backup paths are lost anyway, so this doesn't provide a solution. |
||
| if ('deviceIdentifier' in props) { | ||
| const query = { | ||
| key: props.deviceIdentifier.key, | ||
| platform: props.deviceIdentifier.platform, | ||
| hostname: props.deviceIdentifier.hostname, | ||
| limit: 1, | ||
| offset: 0, | ||
| }; | ||
| const result = await driveServerModule.backup.getDevicesByIdentifier(query); | ||
|
|
||
| if (result.isLeft()) return left(result.getLeft()); | ||
| const { error, data } = await driveServerModule.backup.getDevicesByIdentifier({ query }); | ||
| if (error) return { error }; | ||
|
|
||
| const devices = result.getRight(); | ||
| if (devices.length === 0) return right(null); | ||
| if (devices.length > 1) return left(new Error('Multiple devices found for the same identifier')); | ||
| if (data.length === 0) { | ||
| return { error: new BackupError('Device not found with given identifier', 'NOT_FOUND') }; | ||
| } | ||
|
|
||
| return right(devices[0]); | ||
| return { data: data[0] }; | ||
| } else { | ||
| const deviceResult = | ||
| 'uuid' in props | ||
| ? await driveServerModule.backup.getDevice(props.uuid) | ||
| : await driveServerModule.backup.getDeviceById(props.legacyId); | ||
|
|
||
| if (deviceResult.isLeft()) return left(deviceResult.getLeft()); | ||
| if (deviceResult.isLeft()) return { error: deviceResult.getLeft() }; | ||
|
|
||
| return right(deviceResult.getRight()); | ||
| return { data: deviceResult.getRight() }; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a device exists using the provided identifier. | ||
| * @param props - Union type object containing either: | ||
| * - { uuid: string } for UUID-based lookup | ||
| * - { legacyId: string } for legacy ID-based lookup | ||
| * - { deviceIdentifier: DeviceIdentifierDTO } for lookup by device identifier (key, platform, hostname) | ||
| * | ||
| * The function will automatically select the correct lookup method based on the provided property. | ||
| * | ||
| * @returns Either<Error, Device | null> - Right(Device) if found, Right(null) if not found, Left(Error) if error | ||
| */ | ||
| export async function fetchDevice(props: FetchDeviceProps): Promise<Either<Error, Device | null>> { | ||
| const getDeviceEither = await getDeviceByProps(props); | ||
|
|
||
| if (getDeviceEither.isRight()) { | ||
| const device = getDeviceEither.getRight(); | ||
| if (device && !device.removed) { | ||
| logger.debug({ | ||
| tag: 'BACKUPS', | ||
| msg: '[DEVICE] Found device', | ||
| device: device.name, | ||
| }); | ||
| return right(device); | ||
| } | ||
| } | ||
|
|
||
| if (getDeviceEither.isLeft()) { | ||
| const error = getDeviceEither.getLeft(); | ||
| export async function fetchDevice(props: FetchDeviceProps) { | ||
| const { error, data } = await getDeviceByProps(props); | ||
|
|
||
| if (error) { | ||
| if (error instanceof BackupError && error.code === 'NOT_FOUND') { | ||
| const msg = 'Device not found'; | ||
| logger.debug({ tag: 'BACKUPS', msg: `[DEVICE] ${msg}` }); | ||
| addUnknownDeviceIssue(new Error(msg)); | ||
| return right(null); | ||
| } | ||
|
|
||
| if (error instanceof BackupError && error.code === 'FORBIDDEN') { | ||
| const msg = 'Device request returned forbidden'; | ||
| logger.debug({ tag: 'BACKUPS', msg: `[DEVICE] ${msg}` }); | ||
| addUnknownDeviceIssue(new Error(msg)); | ||
| return right(null); | ||
| } | ||
|
|
||
| logger.error({ tag: 'BACKUPS', msg: '[DEVICE] Error fetching device', error: error.name }); | ||
| return left(error); | ||
| return { error }; | ||
| } | ||
|
|
||
| return right(null); | ||
| return { data }; | ||
| } | ||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically
backupsBucketis always defined so we shouldn't do this anymore. We had the same code on windows and we deleted it. Check if that's true and you can simplify this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran some tests and confirmed that the backupsBucket does not exist for new users until they create their first device, so this line is still necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh shit, then we have a bug on windows. Thanks for the info :)