fix(mail): stop Gmail refusing the search, and stop calling that "hittade inget" (#1521)
Pressing Leta produced mails=25, documents=0 on a real two-mailbox run. Nothing was found because nothing was searched: every request came back 429 "Too many concurrent requests for user". Two bugs, and the second is the one that matters. The search fanned out with Promise.all over every message id at once, one Gmail request per message, per connection. Gmail enforces a per-user concurrency ceiling as well as a daily quota, and this sailed past it long before any volume worth worrying about. It now runs through a pool of five per connection, which is comfortably under and still finishes a page of results in a couple of round trips. The catch turned each refusal into an empty array, with a comment saying one mailbox's failure must not become the company's. Right instinct, wrong consequence: an empty array is also what an empty mailbox returns, and the manual hunt loop stops on fetched === 0 because that is its signal for "the mailboxes hold nothing more for what is open". So a rate-limited search told the user their receipts do not exist, and stopped looking. searchFailureCount() now separates "could not look" from "nothing there". The run route reports it, and the loop treats a pass with failures as failed rather than finished, so pressing again is the obvious next move instead of a pointless one. This is the failure this feature exists to catch, happening inside the feature: silence that reads as an answer. Restoring the unbounded fan-out fails one test; removing the failure counter fails three. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Opus 5
parent
709c0c817a
commit
f2d9e98af3
@@ -0,0 +1,118 @@
|
||||
/**
|
||||
* Gmail refuses before any quota is near.
|
||||
*
|
||||
* The search fanned out one request per message id at once. Gmail answers 429
|
||||
* "Too many concurrent requests for user" to that, and the catch turned the
|
||||
* refusal into an empty result — which is indistinguishable from a mailbox that
|
||||
* genuinely holds nothing. A real run against two connections produced
|
||||
* `mails=25 documents=0`, and the client loop read the zero as "nothing left to
|
||||
* find" and stopped. The user was told there were no receipts by a search that
|
||||
* never happened.
|
||||
*
|
||||
* Two things must hold: the fan-out stays under the ceiling, and a refusal is
|
||||
* distinguishable from an empty mailbox.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
const searchMessageIds = vi.fn()
|
||||
const getMessageSummary = vi.fn()
|
||||
const getAccessToken = vi.fn()
|
||||
const listActiveConnections = vi.fn()
|
||||
const touchSearched = vi.fn()
|
||||
|
||||
vi.mock('@/lib/auth/api-keys', () => ({ createServiceClientNoCookies: () => ({}) }))
|
||||
vi.mock('../gmail-client', () => ({
|
||||
searchMessageIds: (...a: unknown[]) => searchMessageIds(...a),
|
||||
getMessageSummary: (...a: unknown[]) => getMessageSummary(...a),
|
||||
fetchAttachmentBytes: vi.fn(),
|
||||
clearMessageCache: vi.fn(),
|
||||
describeAttachment: vi.fn(),
|
||||
}))
|
||||
vi.mock('../google-oauth', () => ({ isGoogleMailConfigured: () => true }))
|
||||
vi.mock('../connections', () => ({
|
||||
getAccessToken: (...a: unknown[]) => getAccessToken(...a),
|
||||
listActiveConnections: (...a: unknown[]) => listActiveConnections(...a),
|
||||
touchSearched: (...a: unknown[]) => touchSearched(...a),
|
||||
}))
|
||||
|
||||
const { GmailSearchService } = await import('../search-service')
|
||||
|
||||
function connection(id: string) {
|
||||
return { id, email_address: `${id}@example.test`, provider: 'gmail', status: 'active' }
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
getAccessToken.mockResolvedValue('token')
|
||||
listActiveConnections.mockResolvedValue([connection('c1')])
|
||||
touchSearched.mockResolvedValue(undefined)
|
||||
})
|
||||
|
||||
describe('GmailSearchService.search', () => {
|
||||
it('never has more than a handful of summary requests in flight', async () => {
|
||||
// The ceiling is Gmail's, not ours: exceeding it fails the whole search.
|
||||
let inFlight = 0
|
||||
let peak = 0
|
||||
searchMessageIds.mockResolvedValue(Array.from({ length: 40 }, (_, i) => `m${i}`))
|
||||
getMessageSummary.mockImplementation(async () => {
|
||||
inFlight++
|
||||
peak = Math.max(peak, inFlight)
|
||||
await new Promise((r) => setTimeout(r, 1))
|
||||
inFlight--
|
||||
return { messageId: 'm', subject: 'Kvitto', from: 'a@b.c' }
|
||||
})
|
||||
|
||||
const svc = new GmailSearchService()
|
||||
await svc.search('company-1', { merchant: 'x', amount: 1, currency: 'SEK', date: '2026-08-01' })
|
||||
|
||||
expect(getMessageSummary).toHaveBeenCalledTimes(40)
|
||||
expect(peak).toBeLessThanOrEqual(5)
|
||||
})
|
||||
|
||||
it('reports a refused search instead of passing it off as an empty mailbox', async () => {
|
||||
searchMessageIds.mockRejectedValue(new Error('Gmail 429: Too many concurrent requests for user.'))
|
||||
|
||||
const svc = new GmailSearchService()
|
||||
const out = await svc.search('company-1', { merchant: 'x', amount: 1, currency: 'SEK', date: '2026-08-01' })
|
||||
|
||||
// No candidates either way; the count is the only thing that separates
|
||||
// "could not look" from "nothing there".
|
||||
expect(out).toEqual([])
|
||||
expect(svc.searchFailureCount()).toBe(1)
|
||||
})
|
||||
|
||||
it('counts a genuinely empty mailbox as no failure', async () => {
|
||||
searchMessageIds.mockResolvedValue([])
|
||||
const svc = new GmailSearchService()
|
||||
const out = await svc.search('company-1', { merchant: 'x', amount: 1, currency: 'SEK', date: '2026-08-01' })
|
||||
expect(out).toEqual([])
|
||||
expect(svc.searchFailureCount()).toBe(0)
|
||||
})
|
||||
|
||||
it('lets one refused mailbox shrink the search without hiding the others', async () => {
|
||||
listActiveConnections.mockResolvedValue([connection('c1'), connection('c2')])
|
||||
let call = 0
|
||||
searchMessageIds.mockImplementation(async () => {
|
||||
call++
|
||||
if (call === 1) throw new Error('Gmail 429')
|
||||
return ['m1']
|
||||
})
|
||||
getMessageSummary.mockResolvedValue({ messageId: 'm1', subject: 'Kvitto', from: 'a@b.c' })
|
||||
|
||||
const svc = new GmailSearchService()
|
||||
const out = await svc.search('company-1', { merchant: 'x', amount: 1, currency: 'SEK', date: '2026-08-01' })
|
||||
|
||||
expect(out.length).toBe(1)
|
||||
expect(svc.searchFailureCount()).toBe(1)
|
||||
})
|
||||
|
||||
it('starts each search from zero failures', async () => {
|
||||
searchMessageIds.mockRejectedValueOnce(new Error('Gmail 429')).mockResolvedValue([])
|
||||
const svc = new GmailSearchService()
|
||||
const q = { merchant: 'x', amount: 1, currency: 'SEK', date: '2026-08-01' }
|
||||
await svc.search('company-1', q)
|
||||
expect(svc.searchFailureCount()).toBe(1)
|
||||
await svc.search('company-1', q)
|
||||
expect(svc.searchFailureCount()).toBe(0)
|
||||
})
|
||||
})
|
||||
@@ -43,7 +43,45 @@ function canonicalOrigin(): string {
|
||||
return process.env.NEXT_PUBLIC_APP_URL?.trim() || 'http://localhost:3000'
|
||||
}
|
||||
|
||||
/**
|
||||
* How many message summaries to pull at once, per connection.
|
||||
*
|
||||
* Gmail enforces a per-user concurrency ceiling, not just a daily quota, and
|
||||
* answers 429 "Too many concurrent requests for user" well below any volume
|
||||
* this app generates. Fanning out over every id at once reliably tripped it and
|
||||
* returned an empty search, which is indistinguishable from a mailbox holding
|
||||
* nothing. Five is comfortably under the ceiling and still finishes a page of
|
||||
* results in a couple of round trips.
|
||||
*/
|
||||
const GMAIL_SUMMARY_CONCURRENCY = 5
|
||||
|
||||
/** Map with a bounded worker pool, preserving input order. */
|
||||
async function mapWithConcurrency<T, R>(
|
||||
items: readonly T[],
|
||||
limit: number,
|
||||
fn: (item: T) => Promise<R>,
|
||||
): Promise<R[]> {
|
||||
const out = new Array<R>(items.length)
|
||||
let next = 0
|
||||
const workers = Array.from({ length: Math.min(limit, items.length) }, async () => {
|
||||
for (;;) {
|
||||
const i = next++
|
||||
if (i >= items.length) return
|
||||
out[i] = await fn(items[i])
|
||||
}
|
||||
})
|
||||
await Promise.all(workers)
|
||||
return out
|
||||
}
|
||||
|
||||
export class GmailSearchService implements MailSearchService {
|
||||
/**
|
||||
* Connections whose search threw during the last search() call. A refused
|
||||
* mailbox returns no candidates, exactly like an empty one, so without this
|
||||
* the run reports "nothing found" about a mailbox it never managed to read.
|
||||
*/
|
||||
private failures = 0
|
||||
|
||||
isConfigured(): boolean {
|
||||
return isGoogleMailConfigured()
|
||||
}
|
||||
@@ -60,6 +98,7 @@ export class GmailSearchService implements MailSearchService {
|
||||
async search(companyId: string, query: MailSearchQuery): Promise<MailCandidate[]> {
|
||||
if (!this.isConfigured()) return []
|
||||
|
||||
this.failures = 0
|
||||
const supabase = createServiceClientNoCookies()
|
||||
const connections = await listActiveConnections(supabase, companyId)
|
||||
if (connections.length === 0) return []
|
||||
@@ -68,6 +107,8 @@ export class GmailSearchService implements MailSearchService {
|
||||
|
||||
// Mailboxes are searched in parallel: the work is read-only, so there is
|
||||
// nothing to serialise, and one slow account should not delay the rest.
|
||||
// The per-message fan-out inside searchOne is throttled, which is where
|
||||
// Gmail's per-user concurrency ceiling actually bites.
|
||||
const perConnection = await Promise.all(
|
||||
connections.map((connection) => this.searchOne(supabase, connection, q, query.limit)),
|
||||
)
|
||||
@@ -89,17 +130,24 @@ export class GmailSearchService implements MailSearchService {
|
||||
const ids = await searchMessageIds(accessToken, q, limit)
|
||||
if (ids.length === 0) return []
|
||||
|
||||
const summaries = await Promise.all(
|
||||
ids.map((id) =>
|
||||
getMessageSummary(accessToken, id, connection.id, connection.email_address),
|
||||
),
|
||||
// One request per message, but not all at once. Gmail answers 429
|
||||
// "Too many concurrent requests for user" long before any daily quota is
|
||||
// near, and a whole search can come back empty because of it. The failure
|
||||
// used to look exactly like an empty mailbox, so the honest fix is to
|
||||
// stop provoking it rather than to report it more loudly.
|
||||
const summaries = await mapWithConcurrency(ids, GMAIL_SUMMARY_CONCURRENCY, (id) =>
|
||||
getMessageSummary(accessToken, id, connection.id, connection.email_address),
|
||||
)
|
||||
await touchSearched(supabase, connection.id)
|
||||
|
||||
// Cheap pre-filter before anything expensive looks at these.
|
||||
return summaries.filter((c) => looksLikeReceipt(c.subject, c.from))
|
||||
} catch (error) {
|
||||
// Never let one mailbox's failure surface as the company's failure.
|
||||
// Never let one mailbox's failure surface as the company's failure: the
|
||||
// other mailboxes still have answers. But a refused search is not an
|
||||
// empty one, and the caller has to be able to tell them apart, or
|
||||
// "hittade inget" gets said about a mailbox nobody managed to read.
|
||||
this.failures += 1
|
||||
log.warn('gmail search failed for connection', {
|
||||
connectionId: connection.id,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
@@ -108,6 +156,11 @@ export class GmailSearchService implements MailSearchService {
|
||||
}
|
||||
}
|
||||
|
||||
/** How many connections refused the last search. */
|
||||
searchFailureCount(): number {
|
||||
return this.failures
|
||||
}
|
||||
|
||||
async fetchAttachment(
|
||||
connectionId: string,
|
||||
messageId: string,
|
||||
|
||||
Reference in New Issue
Block a user