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:
Pujit Mehrotra
2025-12-18 08:51:01 -05:00
committed by GitHub
parent d13a1f6174
commit 8b155d1f1c
4 changed files with 118 additions and 14 deletions

View File

@@ -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');
});
});
});

View File

@@ -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">

View File

@@ -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",

View File

@@ -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,