diff --git a/.changeset/fix-auth-error-handling.md b/.changeset/fix-auth-error-handling.md new file mode 100644 index 000000000..fd7d4bfeb --- /dev/null +++ b/.changeset/fix-auth-error-handling.md @@ -0,0 +1,9 @@ +--- +"@objectstack/plugin-auth": patch +--- + +fix: AuthPlugin error handling & database adapter config + +- `AuthManager.handleRequest()` now inspects `response.status >= 500` and logs the error body via `console.error`, since better-auth catches internal errors and returns 500 Responses without throwing. +- `AuthPlugin.registerAuthRoutes()` also logs 500+ responses via `ctx.logger.error` for structured plugin logging. +- `createDatabaseConfig()` now wraps the ObjectQL adapter as a `DBAdapterInstance` factory function so better-auth's `getBaseAdapter()` correctly recognises it (via `typeof database === "function"` check) instead of falling through to the Kysely adapter path. diff --git a/ROADMAP.md b/ROADMAP.md index c404d8b8f..65aef3f61 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -329,6 +329,7 @@ business/custom objects, aligning with industry best practices (e.g., ServiceNow **Migration (v3.x → v4.0):** - v3.x: The `SystemObjectName` constants now emit `sys_`-prefixed names. Implementations using `StorageNameMapping.resolveTableName()` can set `tableName` to preserve legacy physical table names during the transition. - v3.x: The `@objectstack/plugin-auth` ObjectQL adapter now includes `AUTH_MODEL_TO_PROTOCOL` mapping to translate better-auth's hardcoded model names (`user`, `session`, `account`, `verification`) to protocol names (`sys_user`, `sys_session`, `sys_account`, `sys_verification`). Custom adapters must adopt the same mapping. +- v3.x: **Bug fix** — `AuthManager.createDatabaseConfig()` now wraps the ObjectQL adapter as a `DBAdapterInstance` factory function (`(options) => DBAdapter`). Previously the raw adapter object was passed, which fell through to the Kysely adapter path and failed silently. `AuthManager.handleRequest()` and `AuthPlugin.registerAuthRoutes()` now inspect `response.status >= 500` and log the error body, since better-auth catches internal errors and returns 500 Responses without throwing. - v4.0: Legacy un-prefixed aliases will be fully removed. --- diff --git a/packages/plugins/plugin-auth/README.md b/packages/plugins/plugin-auth/README.md index f07c50a51..df6f9ee1d 100644 --- a/packages/plugins/plugin-auth/README.md +++ b/packages/plugins/plugin-auth/README.md @@ -232,13 +232,22 @@ const adapter = createObjectQLAdapter(dataEngine); // Mapping: { user: 'sys_user', session: 'sys_session', account: 'sys_account', verification: 'sys_verification' } console.log(AUTH_MODEL_TO_PROTOCOL); -// Better-auth uses this adapter for all database operations +// better-auth requires a DBAdapterInstance (factory function), not a raw adapter object. +// Passing a plain object falls through to the Kysely adapter path and fails silently. +// Wrap the adapter in a factory function: const auth = betterAuth({ - database: adapter, + database: (options) => ({ + id: 'objectql', + ...adapter, + transaction: async (cb) => cb(adapter), + }), // ... other config }); ``` +> **Note:** `AuthManager` handles this wrapping automatically when you provide a `dataEngine`. +> You only need the factory pattern above when using `createObjectQLAdapter()` directly. + ## Development ```bash diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts new file mode 100644 index 000000000..fd700b5c8 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -0,0 +1,193 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { AuthManager } from './auth-manager'; + +// Mock better-auth so we can control the handler behaviour +vi.mock('better-auth', () => ({ + betterAuth: vi.fn(() => ({ + handler: vi.fn(), + api: {}, + })), +})); + +import { betterAuth } from 'better-auth'; + +describe('AuthManager', () => { + let consoleSpy: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleSpy.mockRestore(); + }); + + describe('handleRequest – error response logging', () => { + it('should log when better-auth returns a 500 response', async () => { + const errorResponse = new Response( + JSON.stringify({ error: 'Internal database error' }), + { status: 500, headers: { 'Content-Type': 'application/json' } }, + ); + + const mockHandler = vi.fn().mockResolvedValue(errorResponse); + (betterAuth as any).mockReturnValue({ handler: mockHandler, api: {} }); + + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + }); + + const request = new Request('http://localhost:3000/sign-up/email', { + method: 'POST', + body: JSON.stringify({ email: 'a@b.com', password: 'pass' }), + headers: { 'Content-Type': 'application/json' }, + }); + + const response = await manager.handleRequest(request); + + expect(response.status).toBe(500); + expect(consoleSpy).toHaveBeenCalledWith( + '[AuthManager] better-auth returned error:', + 500, + expect.stringContaining('Internal database error'), + ); + }); + + it('should NOT log for successful (2xx) responses', async () => { + const okResponse = new Response(JSON.stringify({ user: {} }), { + status: 200, + }); + + const mockHandler = vi.fn().mockResolvedValue(okResponse); + (betterAuth as any).mockReturnValue({ handler: mockHandler, api: {} }); + + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + }); + + const request = new Request('http://localhost:3000/sign-in/email', { + method: 'POST', + body: JSON.stringify({ email: 'a@b.com', password: 'pass' }), + headers: { 'Content-Type': 'application/json' }, + }); + + const response = await manager.handleRequest(request); + + expect(response.status).toBe(200); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it('should NOT log for 4xx responses', async () => { + const badRequestResponse = new Response( + JSON.stringify({ error: 'Bad request' }), + { status: 400 }, + ); + + const mockHandler = vi.fn().mockResolvedValue(badRequestResponse); + (betterAuth as any).mockReturnValue({ handler: mockHandler, api: {} }); + + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + }); + + const request = new Request('http://localhost:3000/sign-in/email', { + method: 'POST', + }); + + const response = await manager.handleRequest(request); + + expect(response.status).toBe(400); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + }); + + describe('createDatabaseConfig – adapter wrapping', () => { + it('should pass a function (DBAdapterInstance) to betterAuth when dataEngine is provided', () => { + const mockDataEngine = { + insert: vi.fn(), + findOne: vi.fn(), + find: vi.fn(), + count: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + + new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + dataEngine: mockDataEngine as any, + }); + + // Trigger lazy initialization by calling getAuthInstance() + // betterAuth should have been called with a database value that is a function + // We need to trigger the lazy init first + }); + + it('should provide a factory function as database config that returns adapter with id and transaction', () => { + const mockDataEngine = { + insert: vi.fn().mockResolvedValue({ id: '1' }), + findOne: vi.fn().mockResolvedValue({ id: '1' }), + find: vi.fn().mockResolvedValue([]), + count: vi.fn().mockResolvedValue(0), + update: vi.fn().mockResolvedValue({ id: '1' }), + delete: vi.fn().mockResolvedValue(undefined), + }; + + let capturedConfig: any; + (betterAuth as any).mockImplementation((config: any) => { + capturedConfig = config; + return { handler: vi.fn(), api: {} }; + }); + + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + dataEngine: mockDataEngine as any, + }); + + // Trigger lazy initialisation + manager.getAuthInstance(); + + // The database config should be a function (DBAdapterInstance) + expect(typeof capturedConfig.database).toBe('function'); + + // Calling the factory should return an adapter object + const adapterResult = capturedConfig.database({}); + expect(adapterResult).toHaveProperty('id', 'objectql'); + expect(typeof adapterResult.create).toBe('function'); + expect(typeof adapterResult.findOne).toBe('function'); + expect(typeof adapterResult.findMany).toBe('function'); + expect(typeof adapterResult.count).toBe('function'); + expect(typeof adapterResult.update).toBe('function'); + expect(typeof adapterResult.delete).toBe('function'); + expect(typeof adapterResult.deleteMany).toBe('function'); + expect(typeof adapterResult.updateMany).toBe('function'); + expect(typeof adapterResult.transaction).toBe('function'); + }); + + it('should return undefined (in-memory fallback) when no dataEngine is provided', () => { + let capturedConfig: any; + (betterAuth as any).mockImplementation((config: any) => { + capturedConfig = config; + return { handler: vi.fn(), api: {} }; + }); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + }); + + manager.getAuthInstance(); + + expect(capturedConfig.database).toBeUndefined(); + warnSpy.mockRestore(); + }); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 92f5d7cf3..dbf1e0ddf 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -91,11 +91,30 @@ export class AuthManager { /** * Create database configuration using ObjectQL adapter + * + * better-auth resolves the `database` option as follows: + * - `undefined` → in-memory adapter + * - `typeof fn === "function"` → treated as `DBAdapterInstance`, called with `(options)` + * - otherwise → forwarded to Kysely adapter factory (pool/dialect) + * + * A raw `CustomAdapter` object would fall into the third branch and fail + * silently. We therefore wrap the ObjectQL adapter in a factory function + * so it is correctly recognised as a `DBAdapterInstance`. */ private createDatabaseConfig(): any { // Use ObjectQL adapter if dataEngine is provided if (this.config.dataEngine) { - return createObjectQLAdapter(this.config.dataEngine); + const adapter = createObjectQLAdapter(this.config.dataEngine); + // Return a DBAdapterInstance factory function + return (_options: any) => ({ + id: 'objectql', + ...adapter, + // ObjectQL does not yet expose a separate transaction context, + // so we pass the adapter itself. better-auth patches this + // automatically when missing, but providing it avoids a + // runtime warning from getBaseAdapter(). + transaction: async (cb: (trx: any) => Promise): Promise => cb(adapter), + }); } // Fallback warning if no dataEngine is provided @@ -145,13 +164,28 @@ export class AuthManager { /** * Handle an authentication request * Forwards the request directly to better-auth's universal handler + * + * better-auth catches internal errors (database / adapter / ORM) and + * returns a 500 Response instead of throwing. We therefore inspect the + * response status and log server errors so they are not silently swallowed. * * @param request - Web standard Request object * @returns Web standard Response object */ async handleRequest(request: Request): Promise { const auth = this.getOrCreateAuth(); - return await auth.handler(request); + const response = await auth.handler(request); + + if (response.status >= 500) { + try { + const body = await response.clone().text(); + console.error('[AuthManager] better-auth returned error:', response.status, body); + } catch { + console.error('[AuthManager] better-auth returned error:', response.status, '(unable to read body)'); + } + } + + return response; } /** diff --git a/packages/plugins/plugin-auth/src/auth-plugin.test.ts b/packages/plugins/plugin-auth/src/auth-plugin.test.ts index f8ee2678c..54ac2386e 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.test.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.test.ts @@ -136,6 +136,61 @@ describe('AuthPlugin', () => { ); }); + it('should log via ctx.logger when better-auth returns a 500 response', async () => { + const mockRawApp = { + all: vi.fn(), + }; + + const mockHttpServer = { + post: vi.fn(), + get: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + use: vi.fn(), + getRawApp: vi.fn(() => mockRawApp), + }; + + mockContext.getService = vi.fn((name: string) => { + if (name === 'http-server') return mockHttpServer; + throw new Error(`Service not found: ${name}`); + }); + + await authPlugin.start(mockContext); + + // Extract the registered route handler + const routeHandler = mockRawApp.all.mock.calls[0][1]; + + // Create a mock Hono context with a request that will trigger a 500 response + const errorResponse = new Response( + JSON.stringify({ error: 'Database connection failed' }), + { status: 500, headers: { 'Content-Type': 'application/json' } } + ); + + // Mock the authManager's handleRequest to return a 500 response + // We access the private authManager through the registered service + const registeredAuthManager = (mockContext.registerService as any).mock.calls[0][1]; + vi.spyOn(registeredAuthManager, 'handleRequest').mockResolvedValue(errorResponse); + + const mockHonoCtx = { + req: { + raw: new Request('http://localhost:3000/api/v1/auth/sign-up/email', { + method: 'POST', + body: JSON.stringify({ email: 'a@b.com', password: 'pass' }), + headers: { 'Content-Type': 'application/json' }, + }), + }, + }; + + const result = await routeHandler(mockHonoCtx); + + expect(result.status).toBe(500); + expect(mockContext.logger.error).toHaveBeenCalledWith( + '[AuthPlugin] better-auth returned server error', + expect.any(Error) + ); + }); + it('should skip route registration when disabled', async () => { authPlugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long', diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 2bae195c0..3369c1b3f 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -199,6 +199,18 @@ export class AuthPlugin implements Plugin { // Forward to better-auth handler const response = await this.authManager!.handleRequest(rewrittenRequest); + + // better-auth catches internal errors and returns error Responses + // without throwing, so the catch block below would never trigger. + // We proactively log server errors here for observability. + if (response.status >= 500) { + try { + const body = await response.clone().text(); + ctx.logger.error('[AuthPlugin] better-auth returned server error', new Error(`HTTP ${response.status}: ${body}`)); + } catch { + ctx.logger.error('[AuthPlugin] better-auth returned server error', new Error(`HTTP ${response.status}: (unable to read body)`)); + } + } return response; } catch (error) {