-
Notifications
You must be signed in to change notification settings - Fork 4
UserManager abstraction added #158
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
base: refactor/hawk-storage
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export type { HawkStorage } from './storages/hawk-storage'; | ||
| export type { UserManager } from './users/user-manager'; | ||
| export { HawkStorageUserManager } from './users/hawk-storage-user-manager'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /** | ||
| * Abstract key–value storage contract used by Hawk internals to persist data across sessions. | ||
| */ | ||
| export interface HawkStorage { | ||
| /** | ||
| * Returns the value associated with the given key, or `null` if none exists. | ||
| * | ||
| * @param key - Storage key to look up. | ||
| */ | ||
| getItem(key: string): string | null | ||
|
|
||
| /** | ||
| * Persists a value under the given key. | ||
| * | ||
| * @param key - Storage key. | ||
| * @param value - Value to store. | ||
| */ | ||
| setItem(key: string, value: string): void | ||
|
|
||
| /** | ||
| * Removes the entry for the given key. | ||
| * | ||
| * @param key - Storage key to remove. | ||
| */ | ||
| removeItem(key: string): void | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { AffectedUser } from '@hawk.so/types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { HawkStorage } from '../storages/hawk-storage'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { UserManager } from './user-manager'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Storage key used to persist the user identifier. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const HAWK_USER_STORAGE_KEY = 'hawk-user-id'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * {@link UserManager} implementation that persists the affected user | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * via an injected {@link HawkStorage} backend. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class HawkStorageUserManager implements UserManager { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
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. its not clear for me why do we need this class? Maybe UserManager can do the same (accept HawkStorage as an argument and implement getUser/setUser etc) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Underlying storage used to read and write the user identifier. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private readonly storage: HawkStorage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param storage - Storage backend to use for persistence. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(storage: HawkStorage) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.storage = storage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** @inheritDoc */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public getUser(): AffectedUser | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const storedId = this.storage.getItem(HAWK_USER_STORAGE_KEY); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (storedId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: storedId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+37
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const storedId = this.storage.getItem(HAWK_USER_STORAGE_KEY); | |
| if (storedId) { | |
| return { | |
| id: storedId, | |
| }; | |
| } | |
| return null; | |
| try { | |
| const storedId = this.storage.getItem(HAWK_USER_STORAGE_KEY); | |
| if (storedId) { | |
| return { | |
| id: storedId, | |
| }; | |
| } | |
| return null; | |
| } catch { | |
| return null; | |
| } |
Copilot
AI
Feb 17, 2026
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.
This method only reconstructs the user object with the id field from storage, losing any other AffectedUser properties that were originally set. This is inconsistent with the setUser method which accepts a full AffectedUser object. See also the comment on the setUser method.
Copilot
AI
Feb 17, 2026
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.
The HawkStorageUserManager only persists the user.id field and loses any other AffectedUser properties (such as name, email, url, image, or any custom fields that AffectedUser might support). When getUser() is called, it only reconstructs the id field. This means if a developer calls setUser with a full user object containing additional properties, those properties will be lost. Consider serializing the entire AffectedUser object as JSON, or document this limitation clearly if it's intentional.
| const storedId = this.storage.getItem(HAWK_USER_STORAGE_KEY); | |
| if (storedId) { | |
| return { | |
| id: storedId, | |
| }; | |
| } | |
| return null; | |
| } | |
| /** @inheritDoc */ | |
| public setUser(user: AffectedUser): void { | |
| this.storage.setItem(HAWK_USER_STORAGE_KEY, user.id); | |
| const storedValue = this.storage.getItem(HAWK_USER_STORAGE_KEY); | |
| if (!storedValue) { | |
| return null; | |
| } | |
| // Try to parse the stored value as a full AffectedUser object. | |
| try { | |
| const parsed = JSON.parse(storedValue) as AffectedUser | null; | |
| if (parsed && typeof parsed === 'object' && 'id' in parsed) { | |
| return parsed; | |
| } | |
| } catch { | |
| // Ignore JSON parsing errors and fall back to legacy behavior below. | |
| } | |
| // Backward compatibility: prior versions stored only the user id as a string. | |
| return { | |
| id: storedValue, | |
| }; | |
| } | |
| /** @inheritDoc */ | |
| public setUser(user: AffectedUser): void { | |
| // Persist the full AffectedUser object so that all properties are retained. | |
| this.storage.setItem(HAWK_USER_STORAGE_KEY, JSON.stringify(user)); |
Copilot
AI
Feb 17, 2026
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.
The setUser() and clear() methods don't handle potential errors from storage operations. If the storage is unavailable or throws an error (e.g., QuotaExceededError or SecurityError), this could crash the application. Consider wrapping the storage operations in a try-catch block and handling errors gracefully (e.g., logging them without crashing).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import type { AffectedUser } from '@hawk.so/types'; | ||
|
|
||
| /** | ||
| * Contract for user identity managers. | ||
| * | ||
| * Implementations are responsible for persisting and retrieving the | ||
| * {@link AffectedUser} that is attached to every error report sent by the catcher. | ||
| */ | ||
| export interface UserManager { | ||
| /** | ||
| * Returns the current affected user, or `null` if none has been set. | ||
| */ | ||
| getUser(): AffectedUser | null | ||
|
|
||
| /** | ||
| * Replaces the stored user with the provided one. | ||
| * | ||
| * @param user - The affected user to persist. | ||
| */ | ||
| setUser(user: AffectedUser): void | ||
|
|
||
| /** | ||
| * Removes any previously stored user data. | ||
| */ | ||
| clear(): void | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||
| import { describe, it, expect, beforeEach, vi } from 'vitest'; | ||||||
| import { HawkStorageUserManager } from '../../src'; | ||||||
| import type { HawkStorage } from '../../src'; | ||||||
|
|
||||||
| describe('StorageUserManager', () => { | ||||||
|
||||||
| describe('StorageUserManager', () => { | |
| describe('HawkStorageUserManager', () => { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| { | ||
| "extends": "./tsconfig.json", | ||
| "compilerOptions": { | ||
| "outDir": null, | ||
| "declaration": false, | ||
| "types": ["vitest/globals"] | ||
| }, | ||
| "include": [ | ||
| "src/**/*", | ||
| "tests/**/*", | ||
| "vitest.config.ts" | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { defineConfig } from 'vitest/config'; | ||
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| globals: true, | ||
| include: ['tests/**/*.test.ts'], | ||
| typecheck: { | ||
| tsconfig: './tsconfig.test.json', | ||
| }, | ||
| coverage: { | ||
| provider: 'v8', | ||
| include: ['src/**/*.ts'], | ||
| }, | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |||||
| import StackParser from './modules/stackParser'; | ||||||
| import type { CatcherMessage, HawkInitialSettings, BreadcrumbsAPI, Transport } from './types'; | ||||||
| import { VueIntegration } from './integrations/vue'; | ||||||
| import { id } from './utils/id'; | ||||||
| import type { | ||||||
| AffectedUser, | ||||||
| EventContext, | ||||||
|
|
@@ -19,6 +18,10 @@ | |||||
| import { ConsoleCatcher } from './addons/consoleCatcher'; | ||||||
| import { BreadcrumbManager } from './addons/breadcrumbs'; | ||||||
| import { validateUser, validateContext, isValidEventPayload } from './utils/validation'; | ||||||
| import type { UserManager } from '@hawk.so/core'; | ||||||
| import { HawkStorageUserManager } from '@hawk.so/core'; | ||||||
| import { HawkLocalStorage } from './storages/hawk-local-storage'; | ||||||
| import { id } from './utils/id'; | ||||||
|
|
||||||
| /** | ||||||
| * Allow to use global VERSION, that will be overwritten by Webpack | ||||||
|
|
@@ -62,11 +65,6 @@ | |||||
| */ | ||||||
| private readonly release: string | undefined; | ||||||
|
|
||||||
| /** | ||||||
| * Current authenticated user | ||||||
| */ | ||||||
| private user: AffectedUser; | ||||||
|
|
||||||
| /** | ||||||
| * Any additional data passed by user for sending with all messages | ||||||
| */ | ||||||
|
|
@@ -111,6 +109,11 @@ | |||||
| */ | ||||||
| private readonly breadcrumbManager: BreadcrumbManager | null; | ||||||
|
|
||||||
| /** | ||||||
| * Current authenticated user manager instance | ||||||
| */ | ||||||
| private readonly userManager: UserManager = new HawkStorageUserManager(new HawkLocalStorage()); | ||||||
|
|
||||||
| /** | ||||||
| * Catcher constructor | ||||||
| * | ||||||
|
|
@@ -126,7 +129,9 @@ | |||||
| this.token = settings.token; | ||||||
| this.debug = settings.debug || false; | ||||||
| this.release = settings.release !== undefined ? String(settings.release) : undefined; | ||||||
| this.setUser(settings.user || Catcher.getGeneratedUser()); | ||||||
| if (settings.user) { | ||||||
| this.setUser(settings.user); | ||||||
| } | ||||||
| this.setContext(settings.context || undefined); | ||||||
| this.beforeSend = settings.beforeSend; | ||||||
| this.disableVueErrorHandler = | ||||||
|
|
@@ -189,27 +194,6 @@ | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Generates user if no one provided via HawkCatcher settings | ||||||
| * After generating, stores user for feature requests | ||||||
| */ | ||||||
| private static getGeneratedUser(): AffectedUser { | ||||||
| let userId: string; | ||||||
| const LOCAL_STORAGE_KEY = 'hawk-user-id'; | ||||||
| const storedId = localStorage.getItem(LOCAL_STORAGE_KEY); | ||||||
|
|
||||||
| if (storedId) { | ||||||
| userId = storedId; | ||||||
| } else { | ||||||
| userId = id(); | ||||||
|
Member
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. where we create the default id now? |
||||||
| localStorage.setItem(LOCAL_STORAGE_KEY, userId); | ||||||
| } | ||||||
|
|
||||||
| return { | ||||||
| id: userId, | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Send test event from client | ||||||
| */ | ||||||
|
|
@@ -272,14 +256,14 @@ | |||||
| return; | ||||||
| } | ||||||
|
|
||||||
| this.user = user; | ||||||
| this.userManager.setUser(user); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Clear current user information (revert to generated user) | ||||||
| * Clear current user information | ||||||
| */ | ||||||
| public clearUser(): void { | ||||||
| this.user = Catcher.getGeneratedUser(); | ||||||
| this.userManager.clear(); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
|
|
@@ -533,7 +517,7 @@ | |||||
| private getIntegrationId(): string { | ||||||
| try { | ||||||
| const decodedIntegrationToken: DecodedIntegrationToken = JSON.parse(atob(this.token)); | ||||||
| const { integrationId } = decodedIntegrationToken; | ||||||
| const {integrationId} = decodedIntegrationToken; | ||||||
|
Check warning on line 520 in packages/javascript/src/catcher.ts
|
||||||
|
||||||
| const {integrationId} = decodedIntegrationToken; | |
| const { integrationId } = decodedIntegrationToken; |
Check warning on line 560 in packages/javascript/src/catcher.ts
GitHub Actions / lint
A space is required before '}'
Copilot
AI
Feb 17, 2026
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.
The getUser() method now always returns a non-null AffectedUser (by creating and persisting one if none exists), which is a semantic change from the previous implementation. While the HawkJavaScriptEvent type allows user to be AffectedUser | null, the user field will now never be null in practice. Consider updating the event type definition to reflect this guarantee (user: AffectedUser) since a user is always generated and present, or document this behavioral change clearly.
Copilot
AI
Feb 17, 2026
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.
The new user management implementation changes behavior: users are now generated lazily on first getUser() call instead of eagerly in the constructor. However, there are no tests in packages/javascript/tests verifying this new integration behavior, such as verifying that a user is automatically created when an error is sent without explicitly setting one. Consider adding tests to verify the lazy user generation behavior and the integration with UserManager.
Check warning on line 631 in packages/javascript/src/catcher.ts
GitHub Actions / lint
A space is required before '}'
Copilot
AI
Feb 17, 2026
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.
The spacing in object destructuring was changed from const { innerWidth, innerHeight } to const {innerWidth, innerHeight} (no spaces). However, other parts of the codebase use spaces in destructuring (e.g., packages/javascript/src/addons/consoleCatcher.ts lines 258-259). This creates inconsistency. Consider keeping the original spacing to maintain consistency with the rest of the codebase.
| const {innerWidth, innerHeight} = window; | |
| const { innerWidth, innerHeight } = window; |
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.
The @hawk.so/core package imports AffectedUser from '@hawk.so/types' but this dependency is not listed in the package.json. This will cause build and runtime errors. Add '@hawk.so/types': 'npm:0.5.8' to the dependencies or peerDependencies section of packages/core/package.json.