mirror of
https://github.com/unraid/api.git
synced 2026-01-01 06:01:18 -06:00
fix: handle race condition between guid loading and license check (#1847)
On errors, a `console.error` message should be emitted from the browser
console, tagged `[ReplaceCheck.check]`.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added retry capability for license eligibility checks with a
contextual "Retry" button that appears in error states.
* **Bug Fixes**
* Fixed license status initialization to correctly default to ready
state.
* Enhanced error messaging with specific messages for different failure
scenarios (missing credentials, access denied, server errors).
* Improved status display handling to prevent potential runtime errors.
* **Localization**
* Added "Retry" text translation.
* **Tests**
* Updated and added tests for reset functionality and error handling.
<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -14,6 +14,10 @@ import { useServerStore } from '~/store/server';
|
|||||||
|
|
||||||
vi.mock('@unraid/shared-callbacks', () => ({}));
|
vi.mock('@unraid/shared-callbacks', () => ({}));
|
||||||
|
|
||||||
|
vi.mock('@unraid/ui', () => ({
|
||||||
|
BrandLoading: {},
|
||||||
|
}));
|
||||||
|
|
||||||
vi.mock('~/composables/services/keyServer', () => ({
|
vi.mock('~/composables/services/keyServer', () => ({
|
||||||
validateGuid: vi.fn(),
|
validateGuid: vi.fn(),
|
||||||
}));
|
}));
|
||||||
@@ -62,7 +66,7 @@ describe('ReplaceRenew Store', () => {
|
|||||||
expect(store.replaceStatus).toBe('ready');
|
expect(store.replaceStatus).toBe('ready');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should initialize with error state when guid is missing', () => {
|
it('should initialize with ready state even when guid is missing', () => {
|
||||||
vi.mocked(useServerStore).mockReturnValueOnce({
|
vi.mocked(useServerStore).mockReturnValueOnce({
|
||||||
guid: undefined,
|
guid: undefined,
|
||||||
keyfile: mockKeyfile,
|
keyfile: mockKeyfile,
|
||||||
@@ -72,7 +76,8 @@ describe('ReplaceRenew Store', () => {
|
|||||||
|
|
||||||
const newStore = useReplaceRenewStore();
|
const newStore = useReplaceRenewStore();
|
||||||
|
|
||||||
expect(newStore.replaceStatus).toBe('error');
|
// Store now always initializes as 'ready' - errors are set when check() is called
|
||||||
|
expect(newStore.replaceStatus).toBe('ready');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -138,6 +143,18 @@ describe('ReplaceRenew Store', () => {
|
|||||||
expect(store.renewStatus).toBe('installing');
|
expect(store.renewStatus).toBe('installing');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should reset all states with reset action', () => {
|
||||||
|
store.setReplaceStatus('error');
|
||||||
|
store.keyLinkedStatus = 'error';
|
||||||
|
store.error = { name: 'Error', message: 'Test error' };
|
||||||
|
|
||||||
|
store.reset();
|
||||||
|
|
||||||
|
expect(store.replaceStatus).toBe('ready');
|
||||||
|
expect(store.keyLinkedStatus).toBe('ready');
|
||||||
|
expect(store.error).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
describe('check action', () => {
|
describe('check action', () => {
|
||||||
const mockResponse = {
|
const mockResponse = {
|
||||||
hasNewerKeyfile: false,
|
hasNewerKeyfile: false,
|
||||||
@@ -326,8 +343,59 @@ describe('ReplaceRenew Store', () => {
|
|||||||
await store.check();
|
await store.check();
|
||||||
|
|
||||||
expect(store.replaceStatus).toBe('error');
|
expect(store.replaceStatus).toBe('error');
|
||||||
|
expect(store.keyLinkedStatus).toBe('error');
|
||||||
expect(console.error).toHaveBeenCalledWith('[ReplaceCheck.check]', testError);
|
expect(console.error).toHaveBeenCalledWith('[ReplaceCheck.check]', testError);
|
||||||
expect(store.error).toEqual(testError);
|
expect(store.error).toEqual({ name: 'Error', message: 'Test error' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should set error when guid is missing during check', async () => {
|
||||||
|
vi.mocked(useServerStore).mockReturnValue({
|
||||||
|
guid: '',
|
||||||
|
keyfile: mockKeyfile,
|
||||||
|
} as unknown as ReturnType<typeof useServerStore>);
|
||||||
|
|
||||||
|
setActivePinia(createPinia());
|
||||||
|
const testStore = useReplaceRenewStore();
|
||||||
|
|
||||||
|
await testStore.check();
|
||||||
|
|
||||||
|
expect(testStore.replaceStatus).toBe('error');
|
||||||
|
expect(testStore.keyLinkedStatus).toBe('error');
|
||||||
|
expect(testStore.error?.message).toBe('Flash GUID required to check replacement status');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should set error when keyfile is missing during check', async () => {
|
||||||
|
vi.mocked(useServerStore).mockReturnValue({
|
||||||
|
guid: mockGuid,
|
||||||
|
keyfile: '',
|
||||||
|
} as unknown as ReturnType<typeof useServerStore>);
|
||||||
|
|
||||||
|
setActivePinia(createPinia());
|
||||||
|
const testStore = useReplaceRenewStore();
|
||||||
|
|
||||||
|
await testStore.check();
|
||||||
|
|
||||||
|
expect(testStore.replaceStatus).toBe('error');
|
||||||
|
expect(testStore.keyLinkedStatus).toBe('error');
|
||||||
|
expect(testStore.error?.message).toBe('Keyfile required to check replacement status');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should provide descriptive error for 403 status', async () => {
|
||||||
|
const error403 = { response: { status: 403 }, message: 'Forbidden' };
|
||||||
|
vi.mocked(validateGuid).mockRejectedValueOnce(error403);
|
||||||
|
|
||||||
|
await store.check();
|
||||||
|
|
||||||
|
expect(store.error?.message).toBe('Access denied - license may be linked to another account');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should provide descriptive error for 500+ status', async () => {
|
||||||
|
const error500 = { response: { status: 500 }, message: 'Server Error' };
|
||||||
|
vi.mocked(validateGuid).mockRejectedValueOnce(error500);
|
||||||
|
|
||||||
|
await store.check();
|
||||||
|
|
||||||
|
expect(store.error?.message).toBe('Key server temporarily unavailable - please try again later');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,8 +1,9 @@
|
|||||||
<script setup lang="ts">
|
<script setup lang="ts">
|
||||||
|
import { computed } from 'vue';
|
||||||
import { useI18n } from 'vue-i18n';
|
import { useI18n } from 'vue-i18n';
|
||||||
import { storeToRefs } from 'pinia';
|
import { storeToRefs } from 'pinia';
|
||||||
|
|
||||||
import { ArrowTopRightOnSquareIcon, KeyIcon } from '@heroicons/vue/24/solid';
|
import { ArrowPathIcon, ArrowTopRightOnSquareIcon, KeyIcon } from '@heroicons/vue/24/solid';
|
||||||
import { Badge, BrandButton } from '@unraid/ui';
|
import { Badge, BrandButton } from '@unraid/ui';
|
||||||
import { DOCS_REGISTRATION_REPLACE_KEY } from '~/helpers/urls';
|
import { DOCS_REGISTRATION_REPLACE_KEY } from '~/helpers/urls';
|
||||||
|
|
||||||
@@ -11,20 +12,30 @@ import { useReplaceRenewStore } from '~/store/replaceRenew';
|
|||||||
const { t } = useI18n();
|
const { t } = useI18n();
|
||||||
const replaceRenewStore = useReplaceRenewStore();
|
const replaceRenewStore = useReplaceRenewStore();
|
||||||
const { replaceStatusOutput } = storeToRefs(replaceRenewStore);
|
const { replaceStatusOutput } = storeToRefs(replaceRenewStore);
|
||||||
|
|
||||||
|
const isError = computed(() => replaceStatusOutput.value?.variant === 'red');
|
||||||
|
const showButton = computed(() => !replaceStatusOutput.value || isError.value);
|
||||||
|
|
||||||
|
const handleCheck = () => {
|
||||||
|
if (isError.value) {
|
||||||
|
replaceRenewStore.reset();
|
||||||
|
}
|
||||||
|
replaceRenewStore.check(true);
|
||||||
|
};
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
<template>
|
<template>
|
||||||
<div class="flex flex-wrap items-center justify-between gap-2">
|
<div class="flex flex-wrap items-center justify-between gap-2">
|
||||||
<BrandButton
|
<BrandButton
|
||||||
v-if="!replaceStatusOutput"
|
v-if="showButton"
|
||||||
:icon="KeyIcon"
|
:icon="isError ? ArrowPathIcon : KeyIcon"
|
||||||
:text="t('registration.replaceCheck.checkEligibility')"
|
:text="isError ? t('common.retry') : t('registration.replaceCheck.checkEligibility')"
|
||||||
class="grow"
|
class="grow"
|
||||||
@click="replaceRenewStore.check"
|
@click="handleCheck"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
<Badge v-else :variant="replaceStatusOutput.variant" :icon="replaceStatusOutput.icon" size="md">
|
<Badge v-else :variant="replaceStatusOutput?.variant" :icon="replaceStatusOutput?.icon" size="md">
|
||||||
{{ t(replaceStatusOutput.text ?? 'Unknown') }}
|
{{ t(replaceStatusOutput?.text ?? 'Unknown') }}
|
||||||
</Badge>
|
</Badge>
|
||||||
|
|
||||||
<span class="inline-flex flex-wrap items-center justify-end gap-2">
|
<span class="inline-flex flex-wrap items-center justify-end gap-2">
|
||||||
|
|||||||
@@ -30,6 +30,7 @@
|
|||||||
"common.installed": "Installed",
|
"common.installed": "Installed",
|
||||||
"common.installing": "Installing",
|
"common.installing": "Installing",
|
||||||
"common.learnMore": "Learn More",
|
"common.learnMore": "Learn More",
|
||||||
|
"common.retry": "Retry",
|
||||||
"common.loading2": "Loading…",
|
"common.loading2": "Loading…",
|
||||||
"common.success": "Success!",
|
"common.success": "Success!",
|
||||||
"common.unknown": "Unknown",
|
"common.unknown": "Unknown",
|
||||||
|
|||||||
@@ -95,9 +95,7 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
|||||||
renewStatus.value = status;
|
renewStatus.value = status;
|
||||||
};
|
};
|
||||||
|
|
||||||
const replaceStatus = ref<'checking' | 'eligible' | 'error' | 'ineligible' | 'ready'>(
|
const replaceStatus = ref<'checking' | 'eligible' | 'error' | 'ineligible' | 'ready'>('ready');
|
||||||
guid.value ? 'ready' : 'error'
|
|
||||||
);
|
|
||||||
const setReplaceStatus = (status: typeof replaceStatus.value) => {
|
const setReplaceStatus = (status: typeof replaceStatus.value) => {
|
||||||
replaceStatus.value = status;
|
replaceStatus.value = status;
|
||||||
};
|
};
|
||||||
@@ -169,11 +167,15 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
|||||||
const check = async (skipCache: boolean = false) => {
|
const check = async (skipCache: boolean = false) => {
|
||||||
if (!guid.value) {
|
if (!guid.value) {
|
||||||
setReplaceStatus('error');
|
setReplaceStatus('error');
|
||||||
|
setKeyLinked('error');
|
||||||
error.value = { name: 'Error', message: 'Flash GUID required to check replacement status' };
|
error.value = { name: 'Error', message: 'Flash GUID required to check replacement status' };
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
if (!keyfile.value) {
|
if (!keyfile.value) {
|
||||||
setReplaceStatus('error');
|
setReplaceStatus('error');
|
||||||
|
setKeyLinked('error');
|
||||||
error.value = { name: 'Error', message: 'Keyfile required to check replacement status' };
|
error.value = { name: 'Error', message: 'Keyfile required to check replacement status' };
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
@@ -240,11 +242,32 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
const catchError = err as WretchError;
|
const catchError = err as WretchError;
|
||||||
setReplaceStatus('error');
|
setReplaceStatus('error');
|
||||||
error.value = catchError?.message ? catchError : { name: 'Error', message: 'Unknown error' };
|
setKeyLinked('error');
|
||||||
|
|
||||||
|
let errorMessage = 'Unknown error';
|
||||||
|
if (catchError?.response?.status === 401) {
|
||||||
|
errorMessage = 'Authentication failed - please sign in again';
|
||||||
|
} else if (catchError?.response?.status === 403) {
|
||||||
|
errorMessage = 'Access denied - license may be linked to another account';
|
||||||
|
} else if (catchError?.response?.status && catchError.response.status >= 500) {
|
||||||
|
errorMessage = 'Key server temporarily unavailable - please try again later';
|
||||||
|
} else if (catchError?.message) {
|
||||||
|
errorMessage = catchError.message;
|
||||||
|
} else if (typeof navigator !== 'undefined' && !navigator.onLine) {
|
||||||
|
errorMessage = 'No internet connection';
|
||||||
|
}
|
||||||
|
|
||||||
|
error.value = { name: 'Error', message: errorMessage };
|
||||||
console.error('[ReplaceCheck.check]', catchError);
|
console.error('[ReplaceCheck.check]', catchError);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const reset = () => {
|
||||||
|
replaceStatus.value = 'ready';
|
||||||
|
keyLinkedStatus.value = 'ready';
|
||||||
|
error.value = null;
|
||||||
|
};
|
||||||
|
|
||||||
return {
|
return {
|
||||||
// state
|
// state
|
||||||
keyLinkedStatus,
|
keyLinkedStatus,
|
||||||
@@ -255,6 +278,7 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
|||||||
// actions
|
// actions
|
||||||
check,
|
check,
|
||||||
purgeValidationResponse,
|
purgeValidationResponse,
|
||||||
|
reset,
|
||||||
setReplaceStatus,
|
setReplaceStatus,
|
||||||
setRenewStatus,
|
setRenewStatus,
|
||||||
error,
|
error,
|
||||||
|
|||||||
Reference in New Issue
Block a user