Skip to content

Commit b4d5758

Browse files
authored
Merge pull request #879 from objectstack-ai/copilot/fix-better-auth-error-handling
2 parents f848d5c + cfaabbb commit b4d5758

7 files changed

Lines changed: 317 additions & 4 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix: AuthPlugin error handling & database adapter config
6+
7+
- `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.
8+
- `AuthPlugin.registerAuthRoutes()` also logs 500+ responses via `ctx.logger.error` for structured plugin logging.
9+
- `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.

ROADMAP.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,7 @@ business/custom objects, aligning with industry best practices (e.g., ServiceNow
329329
**Migration (v3.x → v4.0):**
330330
- 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.
331331
- 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.
332+
- 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.
332333
- v4.0: Legacy un-prefixed aliases will be fully removed.
333334

334335
---

packages/plugins/plugin-auth/README.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,13 +232,22 @@ const adapter = createObjectQLAdapter(dataEngine);
232232
// Mapping: { user: 'sys_user', session: 'sys_session', account: 'sys_account', verification: 'sys_verification' }
233233
console.log(AUTH_MODEL_TO_PROTOCOL);
234234

235-
// Better-auth uses this adapter for all database operations
235+
// better-auth requires a DBAdapterInstance (factory function), not a raw adapter object.
236+
// Passing a plain object falls through to the Kysely adapter path and fails silently.
237+
// Wrap the adapter in a factory function:
236238
const auth = betterAuth({
237-
database: adapter,
239+
database: (options) => ({
240+
id: 'objectql',
241+
...adapter,
242+
transaction: async (cb) => cb(adapter),
243+
}),
238244
// ... other config
239245
});
240246
```
241247

248+
> **Note:** `AuthManager` handles this wrapping automatically when you provide a `dataEngine`.
249+
> You only need the factory pattern above when using `createObjectQLAdapter()` directly.
250+
242251
## Development
243252

244253
```bash
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
4+
import { AuthManager } from './auth-manager';
5+
6+
// Mock better-auth so we can control the handler behaviour
7+
vi.mock('better-auth', () => ({
8+
betterAuth: vi.fn(() => ({
9+
handler: vi.fn(),
10+
api: {},
11+
})),
12+
}));
13+
14+
import { betterAuth } from 'better-auth';
15+
16+
describe('AuthManager', () => {
17+
let consoleSpy: ReturnType<typeof vi.spyOn>;
18+
19+
beforeEach(() => {
20+
vi.clearAllMocks();
21+
consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
22+
});
23+
24+
afterEach(() => {
25+
consoleSpy.mockRestore();
26+
});
27+
28+
describe('handleRequest – error response logging', () => {
29+
it('should log when better-auth returns a 500 response', async () => {
30+
const errorResponse = new Response(
31+
JSON.stringify({ error: 'Internal database error' }),
32+
{ status: 500, headers: { 'Content-Type': 'application/json' } },
33+
);
34+
35+
const mockHandler = vi.fn().mockResolvedValue(errorResponse);
36+
(betterAuth as any).mockReturnValue({ handler: mockHandler, api: {} });
37+
38+
const manager = new AuthManager({
39+
secret: 'test-secret-at-least-32-chars-long',
40+
baseUrl: 'http://localhost:3000',
41+
});
42+
43+
const request = new Request('http://localhost:3000/sign-up/email', {
44+
method: 'POST',
45+
body: JSON.stringify({ email: 'a@b.com', password: 'pass' }),
46+
headers: { 'Content-Type': 'application/json' },
47+
});
48+
49+
const response = await manager.handleRequest(request);
50+
51+
expect(response.status).toBe(500);
52+
expect(consoleSpy).toHaveBeenCalledWith(
53+
'[AuthManager] better-auth returned error:',
54+
500,
55+
expect.stringContaining('Internal database error'),
56+
);
57+
});
58+
59+
it('should NOT log for successful (2xx) responses', async () => {
60+
const okResponse = new Response(JSON.stringify({ user: {} }), {
61+
status: 200,
62+
});
63+
64+
const mockHandler = vi.fn().mockResolvedValue(okResponse);
65+
(betterAuth as any).mockReturnValue({ handler: mockHandler, api: {} });
66+
67+
const manager = new AuthManager({
68+
secret: 'test-secret-at-least-32-chars-long',
69+
baseUrl: 'http://localhost:3000',
70+
});
71+
72+
const request = new Request('http://localhost:3000/sign-in/email', {
73+
method: 'POST',
74+
body: JSON.stringify({ email: 'a@b.com', password: 'pass' }),
75+
headers: { 'Content-Type': 'application/json' },
76+
});
77+
78+
const response = await manager.handleRequest(request);
79+
80+
expect(response.status).toBe(200);
81+
expect(consoleSpy).not.toHaveBeenCalled();
82+
});
83+
84+
it('should NOT log for 4xx responses', async () => {
85+
const badRequestResponse = new Response(
86+
JSON.stringify({ error: 'Bad request' }),
87+
{ status: 400 },
88+
);
89+
90+
const mockHandler = vi.fn().mockResolvedValue(badRequestResponse);
91+
(betterAuth as any).mockReturnValue({ handler: mockHandler, api: {} });
92+
93+
const manager = new AuthManager({
94+
secret: 'test-secret-at-least-32-chars-long',
95+
baseUrl: 'http://localhost:3000',
96+
});
97+
98+
const request = new Request('http://localhost:3000/sign-in/email', {
99+
method: 'POST',
100+
});
101+
102+
const response = await manager.handleRequest(request);
103+
104+
expect(response.status).toBe(400);
105+
expect(consoleSpy).not.toHaveBeenCalled();
106+
});
107+
});
108+
109+
describe('createDatabaseConfig – adapter wrapping', () => {
110+
it('should pass a function (DBAdapterInstance) to betterAuth when dataEngine is provided', () => {
111+
const mockDataEngine = {
112+
insert: vi.fn(),
113+
findOne: vi.fn(),
114+
find: vi.fn(),
115+
count: vi.fn(),
116+
update: vi.fn(),
117+
delete: vi.fn(),
118+
};
119+
120+
new AuthManager({
121+
secret: 'test-secret-at-least-32-chars-long',
122+
baseUrl: 'http://localhost:3000',
123+
dataEngine: mockDataEngine as any,
124+
});
125+
126+
// Trigger lazy initialization by calling getAuthInstance()
127+
// betterAuth should have been called with a database value that is a function
128+
// We need to trigger the lazy init first
129+
});
130+
131+
it('should provide a factory function as database config that returns adapter with id and transaction', () => {
132+
const mockDataEngine = {
133+
insert: vi.fn().mockResolvedValue({ id: '1' }),
134+
findOne: vi.fn().mockResolvedValue({ id: '1' }),
135+
find: vi.fn().mockResolvedValue([]),
136+
count: vi.fn().mockResolvedValue(0),
137+
update: vi.fn().mockResolvedValue({ id: '1' }),
138+
delete: vi.fn().mockResolvedValue(undefined),
139+
};
140+
141+
let capturedConfig: any;
142+
(betterAuth as any).mockImplementation((config: any) => {
143+
capturedConfig = config;
144+
return { handler: vi.fn(), api: {} };
145+
});
146+
147+
const manager = new AuthManager({
148+
secret: 'test-secret-at-least-32-chars-long',
149+
baseUrl: 'http://localhost:3000',
150+
dataEngine: mockDataEngine as any,
151+
});
152+
153+
// Trigger lazy initialisation
154+
manager.getAuthInstance();
155+
156+
// The database config should be a function (DBAdapterInstance)
157+
expect(typeof capturedConfig.database).toBe('function');
158+
159+
// Calling the factory should return an adapter object
160+
const adapterResult = capturedConfig.database({});
161+
expect(adapterResult).toHaveProperty('id', 'objectql');
162+
expect(typeof adapterResult.create).toBe('function');
163+
expect(typeof adapterResult.findOne).toBe('function');
164+
expect(typeof adapterResult.findMany).toBe('function');
165+
expect(typeof adapterResult.count).toBe('function');
166+
expect(typeof adapterResult.update).toBe('function');
167+
expect(typeof adapterResult.delete).toBe('function');
168+
expect(typeof adapterResult.deleteMany).toBe('function');
169+
expect(typeof adapterResult.updateMany).toBe('function');
170+
expect(typeof adapterResult.transaction).toBe('function');
171+
});
172+
173+
it('should return undefined (in-memory fallback) when no dataEngine is provided', () => {
174+
let capturedConfig: any;
175+
(betterAuth as any).mockImplementation((config: any) => {
176+
capturedConfig = config;
177+
return { handler: vi.fn(), api: {} };
178+
});
179+
180+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
181+
182+
const manager = new AuthManager({
183+
secret: 'test-secret-at-least-32-chars-long',
184+
baseUrl: 'http://localhost:3000',
185+
});
186+
187+
manager.getAuthInstance();
188+
189+
expect(capturedConfig.database).toBeUndefined();
190+
warnSpy.mockRestore();
191+
});
192+
});
193+
});

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,30 @@ export class AuthManager {
9191

9292
/**
9393
* Create database configuration using ObjectQL adapter
94+
*
95+
* better-auth resolves the `database` option as follows:
96+
* - `undefined` → in-memory adapter
97+
* - `typeof fn === "function"` → treated as `DBAdapterInstance`, called with `(options)`
98+
* - otherwise → forwarded to Kysely adapter factory (pool/dialect)
99+
*
100+
* A raw `CustomAdapter` object would fall into the third branch and fail
101+
* silently. We therefore wrap the ObjectQL adapter in a factory function
102+
* so it is correctly recognised as a `DBAdapterInstance`.
94103
*/
95104
private createDatabaseConfig(): any {
96105
// Use ObjectQL adapter if dataEngine is provided
97106
if (this.config.dataEngine) {
98-
return createObjectQLAdapter(this.config.dataEngine);
107+
const adapter = createObjectQLAdapter(this.config.dataEngine);
108+
// Return a DBAdapterInstance factory function
109+
return (_options: any) => ({
110+
id: 'objectql',
111+
...adapter,
112+
// ObjectQL does not yet expose a separate transaction context,
113+
// so we pass the adapter itself. better-auth patches this
114+
// automatically when missing, but providing it avoids a
115+
// runtime warning from getBaseAdapter().
116+
transaction: async <R>(cb: (trx: any) => Promise<R>): Promise<R> => cb(adapter),
117+
});
99118
}
100119

101120
// Fallback warning if no dataEngine is provided
@@ -145,13 +164,28 @@ export class AuthManager {
145164
/**
146165
* Handle an authentication request
147166
* Forwards the request directly to better-auth's universal handler
167+
*
168+
* better-auth catches internal errors (database / adapter / ORM) and
169+
* returns a 500 Response instead of throwing. We therefore inspect the
170+
* response status and log server errors so they are not silently swallowed.
148171
*
149172
* @param request - Web standard Request object
150173
* @returns Web standard Response object
151174
*/
152175
async handleRequest(request: Request): Promise<Response> {
153176
const auth = this.getOrCreateAuth();
154-
return await auth.handler(request);
177+
const response = await auth.handler(request);
178+
179+
if (response.status >= 500) {
180+
try {
181+
const body = await response.clone().text();
182+
console.error('[AuthManager] better-auth returned error:', response.status, body);
183+
} catch {
184+
console.error('[AuthManager] better-auth returned error:', response.status, '(unable to read body)');
185+
}
186+
}
187+
188+
return response;
155189
}
156190

157191
/**

packages/plugins/plugin-auth/src/auth-plugin.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,61 @@ describe('AuthPlugin', () => {
136136
);
137137
});
138138

139+
it('should log via ctx.logger when better-auth returns a 500 response', async () => {
140+
const mockRawApp = {
141+
all: vi.fn(),
142+
};
143+
144+
const mockHttpServer = {
145+
post: vi.fn(),
146+
get: vi.fn(),
147+
put: vi.fn(),
148+
delete: vi.fn(),
149+
patch: vi.fn(),
150+
use: vi.fn(),
151+
getRawApp: vi.fn(() => mockRawApp),
152+
};
153+
154+
mockContext.getService = vi.fn((name: string) => {
155+
if (name === 'http-server') return mockHttpServer;
156+
throw new Error(`Service not found: ${name}`);
157+
});
158+
159+
await authPlugin.start(mockContext);
160+
161+
// Extract the registered route handler
162+
const routeHandler = mockRawApp.all.mock.calls[0][1];
163+
164+
// Create a mock Hono context with a request that will trigger a 500 response
165+
const errorResponse = new Response(
166+
JSON.stringify({ error: 'Database connection failed' }),
167+
{ status: 500, headers: { 'Content-Type': 'application/json' } }
168+
);
169+
170+
// Mock the authManager's handleRequest to return a 500 response
171+
// We access the private authManager through the registered service
172+
const registeredAuthManager = (mockContext.registerService as any).mock.calls[0][1];
173+
vi.spyOn(registeredAuthManager, 'handleRequest').mockResolvedValue(errorResponse);
174+
175+
const mockHonoCtx = {
176+
req: {
177+
raw: new Request('http://localhost:3000/api/v1/auth/sign-up/email', {
178+
method: 'POST',
179+
body: JSON.stringify({ email: 'a@b.com', password: 'pass' }),
180+
headers: { 'Content-Type': 'application/json' },
181+
}),
182+
},
183+
};
184+
185+
const result = await routeHandler(mockHonoCtx);
186+
187+
expect(result.status).toBe(500);
188+
expect(mockContext.logger.error).toHaveBeenCalledWith(
189+
'[AuthPlugin] better-auth returned server error',
190+
expect.any(Error)
191+
);
192+
});
193+
139194
it('should skip route registration when disabled', async () => {
140195
authPlugin = new AuthPlugin({
141196
secret: 'test-secret-at-least-32-chars-long',

packages/plugins/plugin-auth/src/auth-plugin.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,18 @@ export class AuthPlugin implements Plugin {
199199

200200
// Forward to better-auth handler
201201
const response = await this.authManager!.handleRequest(rewrittenRequest);
202+
203+
// better-auth catches internal errors and returns error Responses
204+
// without throwing, so the catch block below would never trigger.
205+
// We proactively log server errors here for observability.
206+
if (response.status >= 500) {
207+
try {
208+
const body = await response.clone().text();
209+
ctx.logger.error('[AuthPlugin] better-auth returned server error', new Error(`HTTP ${response.status}: ${body}`));
210+
} catch {
211+
ctx.logger.error('[AuthPlugin] better-auth returned server error', new Error(`HTTP ${response.status}: (unable to read body)`));
212+
}
213+
}
202214

203215
return response;
204216
} catch (error) {

0 commit comments

Comments
 (0)