From 84c3f1787e2798373aed31a540ca4c0766d3c2ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 11:50:17 +0000 Subject: [PATCH 1/4] Initial plan From fac396df291600c441cecf91b9a28b5b8002b7a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 11:59:13 +0000 Subject: [PATCH 2/4] fix: add error logging for better-auth 500 responses and wrap ObjectQL adapter as DBAdapterInstance Bug 1: better-auth handler catches errors internally and returns 500 Response objects without throwing. Added response status inspection in both AuthManager.handleRequest() and AuthPlugin.registerAuthRoutes() to log server errors that would otherwise be silently swallowed. Bug 2: better-auth resolves `database` option by checking `typeof database === "function"` for custom adapters. Wrapped the ObjectQL adapter in a factory function matching the DBAdapterInstance interface so it is correctly recognized instead of falling through to the Kysely adapter path. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> --- .../plugin-auth/src/auth-manager.test.ts | 193 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 34 ++- .../plugin-auth/src/auth-plugin.test.ts | 55 +++++ .../plugins/plugin-auth/src/auth-plugin.ts | 12 ++ 4 files changed, 292 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/auth-manager.test.ts 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..0d6bc8fc1 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -91,11 +91,26 @@ 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, + transaction: async (cb: (trx: any) => Promise): Promise => cb(adapter), + }); } // Fallback warning if no dataEngine is provided @@ -145,13 +160,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) { From 27db9f36af82e2979d88c3ddc0ead727d27fd862 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:02:13 +0000 Subject: [PATCH 3/4] docs: add transaction comment clarifying ObjectQL limitation per code review Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> --- packages/plugins/plugin-auth/src/auth-manager.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 0d6bc8fc1..dbf1e0ddf 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -109,6 +109,10 @@ export class AuthManager { 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), }); } From cfaabbbacfb04f71ec4813d9c422eda7fef2a830 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:34:31 +0000 Subject: [PATCH 4/4] fix: update README adapter example, add ROADMAP note and changeset - README: Update ObjectQL adapter example to show correct factory-based DBAdapterInstance pattern instead of raw adapter object - ROADMAP: Add v3.x migration note documenting the bug fix - Changeset: Add patch changeset for @objectstack/plugin-auth Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> --- .changeset/fix-auth-error-handling.md | 9 +++++++++ ROADMAP.md | 1 + packages/plugins/plugin-auth/README.md | 13 +++++++++++-- 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-auth-error-handling.md 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