diff --git a/src/reddit/comment-client.ts b/src/reddit/comment-client.ts index bbb60c8..066a111 100644 --- a/src/reddit/comment-client.ts +++ b/src/reddit/comment-client.ts @@ -1,7 +1,13 @@ import type { Logger } from '../utils/logger.js'; import type { RedditRequestScheduler } from './request-coordinator.js'; -const OLD_REDDIT = 'https://old.reddit.com'; +/** + * Every authenticated call is made against this one origin so the page issuing + * it is same-origin: a cross-origin fetch would not send the session cookies, + * and a page still on `about:blank` cannot reach Reddit at all. + */ +export const REDDIT_WRITE_ORIGIN = 'https://old.reddit.com'; +/** The public view, only ever fetched signed out from Node, so CORS never applies. */ const WWW_REDDIT = 'https://www.reddit.com'; /** @@ -25,6 +31,11 @@ export type CommentPostResult = /** Anything else: transient, safe to retry later. */ | { status: 'failed'; reason: string }; +/** Why a modhash lookup failed, so the caller can pick a proportionate response. */ +type ModhashResult = + | { ok: true; modhash: string } + | { ok: false; kind: 'unreachable' | 'signed-out'; reason: string }; + export interface RedditCommentState { fullname: string; score: number | null; @@ -126,16 +137,20 @@ export class RedditCommentClient { } const modhash = await this.ensureModhash(); - if (!modhash) { - return { status: 'session-invalid', reason: 'Reddit did not return a modhash for this session' }; + if (!modhash.ok) { + // Only Reddit actually reporting a signed-out session may invalidate the + // account's auth; anything else is transient and simply retried. + return modhash.kind === 'signed-out' + ? { status: 'session-invalid', reason: modhash.reason } + : { status: 'failed', reason: modhash.reason }; } let response: HttpTextResponse; try { response = await this.scheduler.schedule(() => this.transport.postForm( - `${OLD_REDDIT}/api/comment`, - { thing_id: threadFullname, text, api_type: 'json', uh: modhash }, - { 'X-Modhash': modhash, 'Content-Type': 'application/x-www-form-urlencoded' }, + `${REDDIT_WRITE_ORIGIN}/api/comment`, + { thing_id: threadFullname, text, api_type: 'json', uh: modhash.modhash }, + { 'X-Modhash': modhash.modhash, 'Content-Type': 'application/x-www-form-urlencoded' }, )); } catch (error) { return { status: 'failed', reason: errorMessage(error) }; @@ -188,7 +203,7 @@ export class RedditCommentClient { if (ids.length === 0) return []; if (ids.length > 100) throw new Error('Reddit accepts at most 100 fullnames per info lookup'); - const url = `${WWW_REDDIT}/api/info.json?id=${ids.join(',')}&raw_json=1`; + const url = `${REDDIT_WRITE_ORIGIN}/api/info.json?id=${ids.join(',')}&raw_json=1`; const response = await this.scheduler.schedule(() => this.transport.get(url)); if (response.status !== 200) { throw new Error(`Reddit returned HTTP ${response.status} for comment info`); @@ -232,28 +247,60 @@ export class RedditCommentClient { this.modhash = null; } - private async ensureModhash(): Promise { - if (this.modhash) return this.modhash; + /** + * Resolves this session's modhash, distinguishing "we could not reach Reddit" + * from "Reddit says this session is not signed in". + * + * The distinction matters beyond growth: a `session-invalid` result + * invalidates the account's shared auth context, which would disrupt outreach + * and chat for that account. A network or browser failure must never do that. + */ + private async ensureModhash(): Promise { + if (this.modhash) return { ok: true, modhash: this.modhash }; + let response: HttpTextResponse; try { - const response = await this.scheduler.schedule( - () => this.transport.get(`${OLD_REDDIT}/api/me.json`), + response = await this.scheduler.schedule( + () => this.transport.get(`${REDDIT_WRITE_ORIGIN}/api/me.json`), ); - if (response.status !== 200) { - this.logger.warn(`Reddit returned HTTP ${response.status} for the session identity check`); - return null; - } - const payload = parseJson(response.body); - const data = payload && typeof payload === 'object' - ? (payload as { data?: { modhash?: unknown } }).data - : undefined; - const modhash = typeof data?.modhash === 'string' ? data.modhash : ''; - if (!modhash) return null; - this.modhash = modhash; - return modhash; } catch (error) { - this.logger.warn('Failed to read the Reddit session modhash:', error); - return null; + this.logger.warn('Could not reach Reddit to read the session modhash:', error); + return { + ok: false, + kind: 'unreachable', + reason: `Could not reach Reddit to read the session modhash: ${errorMessage(error)}`, + }; } + + if (response.status === 401 || response.status === 403) { + return { + ok: false, + kind: 'signed-out', + reason: `Reddit returned HTTP ${response.status} for the session identity check`, + }; + } + if (response.status !== 200) { + this.logger.warn(`Reddit returned HTTP ${response.status} for the session identity check`); + return { + ok: false, + kind: 'unreachable', + reason: `Reddit returned HTTP ${response.status} for the session identity check`, + }; + } + + const payload = parseJson(response.body); + const data = payload && typeof payload === 'object' + ? (payload as { data?: { modhash?: unknown } }).data + : undefined; + const modhash = typeof data?.modhash === 'string' ? data.modhash : ''; + if (!modhash) { + return { + ok: false, + kind: 'signed-out', + reason: 'Reddit did not return a modhash for this session', + }; + } + this.modhash = modhash; + return { ok: true, modhash }; } private classifyError( @@ -304,34 +351,64 @@ export class RedditCommentClient { * the request carries that profile's cookies and looks like the signed-in user. */ export interface EvaluatingPage { + url(): string; + goto(url: string, options?: { waitUntil?: 'domcontentloaded'; timeout?: number }): Promise; evaluate( fn: (argument: Argument) => Promise, argument: Argument, ): Promise; } +export function isRedditWriteOrigin(value: string): boolean { + try { + return new URL(value).origin === REDDIT_WRITE_ORIGIN; + } catch { + return false; + } +} + export function createBrowserWriteTransport(page: EvaluatingPage): RedditWriteTransport { + /** + * Parks the page on the Reddit origin before it fetches anything. + * + * A freshly opened page sits on `about:blank`, whose opaque origin cannot + * reach Reddit at all — the fetch fails outright with "Failed to fetch". Any + * other origin would make these calls cross-origin, so the session cookies + * would be withheld even if CORS allowed the request. Being on the same + * origin is what makes them authenticated. + */ + const ensureOrigin = async (): Promise => { + if (isRedditWriteOrigin(page.url())) return; + await page.goto(REDDIT_WRITE_ORIGIN, { waitUntil: 'domcontentloaded', timeout: 30_000 }); + }; + return { // The page already carries the profile's cookies and user agent, so the // request needs no explicit identity of its own. - get: (url) => page.evaluate(async ({ target }) => { - const response = await fetch(target, { - credentials: 'include', - headers: { Accept: 'application/json' }, - }); - return { status: response.status, body: await response.text() }; - }, { target: url }), + get: async (url) => { + await ensureOrigin(); + return page.evaluate(async ({ target }) => { + const response = await fetch(target, { + credentials: 'include', + headers: { Accept: 'application/json' }, + }); + return { status: response.status, body: await response.text() }; + }, { target: url }); + }, - postForm: (url, form, headers) => page.evaluate(async ({ target, fields, extraHeaders }) => { - const body = new URLSearchParams(fields).toString(); - const response = await fetch(target, { - method: 'POST', - credentials: 'include', - headers: { Accept: 'application/json', ...extraHeaders }, - body, - }); - return { status: response.status, body: await response.text() }; - }, { target: url, fields: form, extraHeaders: headers ?? {} }), + postForm: async (url, form, headers) => { + await ensureOrigin(); + return page.evaluate(async ({ target, fields, extraHeaders }) => { + const body = new URLSearchParams(fields).toString(); + const response = await fetch(target, { + method: 'POST', + credentials: 'include', + headers: { Accept: 'application/json', ...extraHeaders }, + body, + }); + return { status: response.status, body: await response.text() }; + }, { target: url, fields: form, extraHeaders: headers ?? {} }); + }, }; } diff --git a/tests/reddit-comment-client.test.ts b/tests/reddit-comment-client.test.ts index b25ac3f..9060caf 100644 --- a/tests/reddit-comment-client.test.ts +++ b/tests/reddit-comment-client.test.ts @@ -149,6 +149,58 @@ describe('RedditCommentClient.postComment', () => { expect(transport.posts).toHaveLength(0); }); + describe('when the modhash lookup cannot reach Reddit', () => { + // `session-invalid` invalidates the account's shared auth context, which + // would disrupt outreach and chat too. Only Reddit actually saying we are + // signed out may do that; a transport failure must stay retryable. + it('reports a thrown transport error as retryable, not signed out', async () => { + const transport = fakeTransport({ + get: () => { throw new TypeError('Failed to fetch'); }, + }); + const client = new RedditCommentClient(transport, logger); + + const result = await client.postComment('t3_a', 'cats', 'text'); + + expect(result.status).toBe('failed'); + expect(result).toMatchObject({ reason: expect.stringContaining('Failed to fetch') }); + expect(transport.posts).toHaveLength(0); + }); + + it('reports a 5xx identity check as retryable', async () => { + const client = new RedditCommentClient( + fakeTransport({ get: () => ({ status: 503, body: '' }) }), + logger, + ); + + expect((await client.postComment('t3_a', 'cats', 'text')).status).toBe('failed'); + }); + + it.each([401, 403])('treats HTTP %i on the identity check as signed out', async (status) => { + const client = new RedditCommentClient( + fakeTransport({ get: () => ({ status, body: '' }) }), + logger, + ); + + expect((await client.postComment('t3_a', 'cats', 'text')).status).toBe('session-invalid'); + }); + + it('retries the lookup on the next attempt rather than caching the failure', async () => { + let attempt = 0; + const transport = fakeTransport({ + get: () => { + attempt += 1; + if (attempt === 1) throw new TypeError('Failed to fetch'); + return MODHASH_OK; + }, + post: () => successBody('t1_ok'), + }); + const client = new RedditCommentClient(transport, logger); + + expect((await client.postComment('t3_a', 'cats', 'text')).status).toBe('failed'); + expect((await client.postComment('t3_a', 'cats', 'text')).status).toBe('posted'); + }); + }); + describe('Reddit error codes', () => { it('maps RATELIMIT to an account-scoped wait and cools the shared scheduler', async () => { const extendCooldown = vi.fn(); @@ -408,9 +460,86 @@ describe('RedditCommentClient.fetchPubliclyVisible', () => { }); describe('createBrowserWriteTransport', () => { + /** Mirrors production: a freshly opened Playwright page is on about:blank. */ + function fakePage(initialUrl = 'about:blank') { + const calls: unknown[] = []; + const navigations: string[] = []; + let current = initialUrl; + return { + calls, + navigations, + url: () => current, + goto: async (target: string) => { + navigations.push(target); + current = target; + return null; + }, + evaluate: async ( + fn: (argument: Argument) => Promise, + argument: Argument, + ): Promise => { + calls.push(argument); + const fetchStub = async (url: string, init?: { method?: string; body?: string }) => ({ + status: 200, + text: async () => JSON.stringify({ url, method: init?.method ?? 'GET', body: init?.body }), + }); + const original = globalThis.fetch; + (globalThis as { fetch: unknown }).fetch = fetchStub; + try { + return await fn(argument); + } finally { + (globalThis as { fetch: unknown }).fetch = original; + } + }, + }; + } + + it('parks the page on the Reddit origin before fetching from about:blank', async () => { + const page = fakePage(); + const transport = createBrowserWriteTransport(page); + + await transport.get('https://old.reddit.com/api/me.json'); + + // Without this a fetch from about:blank fails outright with "Failed to fetch", + // and any other origin would withhold the session cookies. + expect(page.navigations).toEqual(['https://old.reddit.com']); + }); + + it('navigates once and reuses the origin for later requests', async () => { + const page = fakePage(); + const transport = createBrowserWriteTransport(page); + + await transport.get('https://old.reddit.com/api/me.json'); + await transport.postForm('https://old.reddit.com/api/comment', { text: 'hi' }); + await transport.get('https://old.reddit.com/api/info.json?id=t1_a'); + + expect(page.navigations).toEqual(['https://old.reddit.com']); + }); + + it('re-parks the page if it wanders to another origin', async () => { + const page = fakePage('https://www.reddit.com/r/cats/'); + const transport = createBrowserWriteTransport(page); + + await transport.postForm('https://old.reddit.com/api/comment', { text: 'hi' }); + + // www is a different origin, so the cookies would not be sent. + expect(page.navigations).toEqual(['https://old.reddit.com']); + }); + + it('does not navigate when already on the write origin', async () => { + const page = fakePage('https://old.reddit.com/r/cats/'); + const transport = createBrowserWriteTransport(page); + + await transport.get('https://old.reddit.com/api/me.json'); + + expect(page.navigations).toEqual([]); + }); + it('runs both verbs inside the page so requests carry the profile session', async () => { const calls: unknown[] = []; const page = { + url: () => 'https://old.reddit.com/', + goto: async () => null, evaluate: async ( fn: (argument: Argument) => Promise, argument: Argument,