mirror of
https://github.com/unraid/api.git
synced 2025-12-31 05:29:48 -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/ui', () => ({
|
||||
BrandLoading: {},
|
||||
}));
|
||||
|
||||
vi.mock('~/composables/services/keyServer', () => ({
|
||||
validateGuid: vi.fn(),
|
||||
}));
|
||||
@@ -62,7 +66,7 @@ describe('ReplaceRenew Store', () => {
|
||||
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({
|
||||
guid: undefined,
|
||||
keyfile: mockKeyfile,
|
||||
@@ -72,7 +76,8 @@ describe('ReplaceRenew Store', () => {
|
||||
|
||||
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');
|
||||
});
|
||||
|
||||
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', () => {
|
||||
const mockResponse = {
|
||||
hasNewerKeyfile: false,
|
||||
@@ -326,8 +343,59 @@ describe('ReplaceRenew Store', () => {
|
||||
await store.check();
|
||||
|
||||
expect(store.replaceStatus).toBe('error');
|
||||
expect(store.keyLinkedStatus).toBe('error');
|
||||
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">
|
||||
import { computed } from 'vue';
|
||||
import { useI18n } from 'vue-i18n';
|
||||
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 { DOCS_REGISTRATION_REPLACE_KEY } from '~/helpers/urls';
|
||||
|
||||
@@ -11,20 +12,30 @@ import { useReplaceRenewStore } from '~/store/replaceRenew';
|
||||
const { t } = useI18n();
|
||||
const replaceRenewStore = useReplaceRenewStore();
|
||||
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>
|
||||
|
||||
<template>
|
||||
<div class="flex flex-wrap items-center justify-between gap-2">
|
||||
<BrandButton
|
||||
v-if="!replaceStatusOutput"
|
||||
:icon="KeyIcon"
|
||||
:text="t('registration.replaceCheck.checkEligibility')"
|
||||
v-if="showButton"
|
||||
:icon="isError ? ArrowPathIcon : KeyIcon"
|
||||
:text="isError ? t('common.retry') : t('registration.replaceCheck.checkEligibility')"
|
||||
class="grow"
|
||||
@click="replaceRenewStore.check"
|
||||
@click="handleCheck"
|
||||
/>
|
||||
|
||||
<Badge v-else :variant="replaceStatusOutput.variant" :icon="replaceStatusOutput.icon" size="md">
|
||||
{{ t(replaceStatusOutput.text ?? 'Unknown') }}
|
||||
<Badge v-else :variant="replaceStatusOutput?.variant" :icon="replaceStatusOutput?.icon" size="md">
|
||||
{{ t(replaceStatusOutput?.text ?? 'Unknown') }}
|
||||
</Badge>
|
||||
|
||||
<span class="inline-flex flex-wrap items-center justify-end gap-2">
|
||||
|
||||
@@ -30,6 +30,7 @@
|
||||
"common.installed": "Installed",
|
||||
"common.installing": "Installing",
|
||||
"common.learnMore": "Learn More",
|
||||
"common.retry": "Retry",
|
||||
"common.loading2": "Loading…",
|
||||
"common.success": "Success!",
|
||||
"common.unknown": "Unknown",
|
||||
|
||||
@@ -95,9 +95,7 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
||||
renewStatus.value = status;
|
||||
};
|
||||
|
||||
const replaceStatus = ref<'checking' | 'eligible' | 'error' | 'ineligible' | 'ready'>(
|
||||
guid.value ? 'ready' : 'error'
|
||||
);
|
||||
const replaceStatus = ref<'checking' | 'eligible' | 'error' | 'ineligible' | 'ready'>('ready');
|
||||
const setReplaceStatus = (status: typeof replaceStatus.value) => {
|
||||
replaceStatus.value = status;
|
||||
};
|
||||
@@ -169,11 +167,15 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
||||
const check = async (skipCache: boolean = false) => {
|
||||
if (!guid.value) {
|
||||
setReplaceStatus('error');
|
||||
setKeyLinked('error');
|
||||
error.value = { name: 'Error', message: 'Flash GUID required to check replacement status' };
|
||||
return;
|
||||
}
|
||||
if (!keyfile.value) {
|
||||
setReplaceStatus('error');
|
||||
setKeyLinked('error');
|
||||
error.value = { name: 'Error', message: 'Keyfile required to check replacement status' };
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -240,11 +242,32 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
||||
} catch (err) {
|
||||
const catchError = err as WretchError;
|
||||
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);
|
||||
}
|
||||
};
|
||||
|
||||
const reset = () => {
|
||||
replaceStatus.value = 'ready';
|
||||
keyLinkedStatus.value = 'ready';
|
||||
error.value = null;
|
||||
};
|
||||
|
||||
return {
|
||||
// state
|
||||
keyLinkedStatus,
|
||||
@@ -255,6 +278,7 @@ export const useReplaceRenewStore = defineStore('replaceRenewCheck', () => {
|
||||
// actions
|
||||
check,
|
||||
purgeValidationResponse,
|
||||
reset,
|
||||
setReplaceStatus,
|
||||
setRenewStatus,
|
||||
error,
|
||||
|
||||
Reference in New Issue
Block a user