fix: park the growth page on Reddit's origin before fetching
Growth could never post. Its page is opened with newPage() and was never navigated, so it sat on about:blank, whose opaque origin cannot fetch Reddit at all — production logged "Failed to read the Reddit session modhash: TypeError: Failed to fetch" on every attempt. Confirmed against real network: the same fetch throws from about:blank and succeeds from https://old.reddit.com. The transport now parks the page on that origin before any request, and re-parks it if the page wanders elsewhere. Being same-origin is also what sends the session cookies, so the authenticated info.json lookup moved to the same host rather than www; the signed-out visibility probe stays on www because it runs from Node, where CORS does not apply. The unit tests could not catch this: they stub the transport, and the earlier browser check navigated the page by hand in the harness — the production path was the one place that never did. The page fake now starts on about:blank like the real thing and asserts the navigation. Also stop a transport failure from invalidating the account session. A modhash lookup that could not reach Reddit returned session-invalid, which calls auth.invalidate() and so would disrupt outreach and chat for that account over a network blip. Only Reddit actually reporting a signed-out session (HTTP 401/403, or a response with no modhash) does that now; anything else is retryable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1ac6ad22ad
commit
f36e93d5be
2 files changed
+248
-42
No files matched your search
+119
-42
@@ -1,7 +1,13 @@
|
|||||||
import type { Logger } from '../utils/logger.js';
|
import type { Logger } from '../utils/logger.js';
|
||||||
import type { RedditRequestScheduler } from './request-coordinator.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';
|
const WWW_REDDIT = 'https://www.reddit.com';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -25,6 +31,11 @@ export type CommentPostResult =
|
|||||||
/** Anything else: transient, safe to retry later. */
|
/** Anything else: transient, safe to retry later. */
|
||||||
| { status: 'failed'; reason: string };
|
| { 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 {
|
export interface RedditCommentState {
|
||||||
fullname: string;
|
fullname: string;
|
||||||
score: number | null;
|
score: number | null;
|
||||||
@@ -126,16 +137,20 @@ export class RedditCommentClient {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const modhash = await this.ensureModhash();
|
const modhash = await this.ensureModhash();
|
||||||
if (!modhash) {
|
if (!modhash.ok) {
|
||||||
return { status: 'session-invalid', reason: 'Reddit did not return a modhash for this session' };
|
// 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;
|
let response: HttpTextResponse;
|
||||||
try {
|
try {
|
||||||
response = await this.scheduler.schedule(() => this.transport.postForm(
|
response = await this.scheduler.schedule(() => this.transport.postForm(
|
||||||
`${OLD_REDDIT}/api/comment`,
|
`${REDDIT_WRITE_ORIGIN}/api/comment`,
|
||||||
{ thing_id: threadFullname, text, api_type: 'json', uh: modhash },
|
{ thing_id: threadFullname, text, api_type: 'json', uh: modhash.modhash },
|
||||||
{ 'X-Modhash': modhash, 'Content-Type': 'application/x-www-form-urlencoded' },
|
{ 'X-Modhash': modhash.modhash, 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||||
));
|
));
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
return { status: 'failed', reason: errorMessage(error) };
|
return { status: 'failed', reason: errorMessage(error) };
|
||||||
@@ -188,7 +203,7 @@ export class RedditCommentClient {
|
|||||||
if (ids.length === 0) return [];
|
if (ids.length === 0) return [];
|
||||||
if (ids.length > 100) throw new Error('Reddit accepts at most 100 fullnames per info lookup');
|
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));
|
const response = await this.scheduler.schedule(() => this.transport.get(url));
|
||||||
if (response.status !== 200) {
|
if (response.status !== 200) {
|
||||||
throw new Error(`Reddit returned HTTP ${response.status} for comment info`);
|
throw new Error(`Reddit returned HTTP ${response.status} for comment info`);
|
||||||
@@ -232,28 +247,60 @@ export class RedditCommentClient {
|
|||||||
this.modhash = null;
|
this.modhash = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
private async ensureModhash(): Promise<string | null> {
|
/**
|
||||||
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<ModhashResult> {
|
||||||
|
if (this.modhash) return { ok: true, modhash: this.modhash };
|
||||||
|
let response: HttpTextResponse;
|
||||||
try {
|
try {
|
||||||
const response = await this.scheduler.schedule(
|
response = await this.scheduler.schedule(
|
||||||
() => this.transport.get(`${OLD_REDDIT}/api/me.json`),
|
() => 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) {
|
} catch (error) {
|
||||||
this.logger.warn('Failed to read the Reddit session modhash:', error);
|
this.logger.warn('Could not reach Reddit to read the session modhash:', error);
|
||||||
return null;
|
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(
|
private classifyError(
|
||||||
@@ -304,34 +351,64 @@ export class RedditCommentClient {
|
|||||||
* the request carries that profile's cookies and looks like the signed-in user.
|
* the request carries that profile's cookies and looks like the signed-in user.
|
||||||
*/
|
*/
|
||||||
export interface EvaluatingPage {
|
export interface EvaluatingPage {
|
||||||
|
url(): string;
|
||||||
|
goto(url: string, options?: { waitUntil?: 'domcontentloaded'; timeout?: number }): Promise<unknown>;
|
||||||
evaluate<Result, Argument>(
|
evaluate<Result, Argument>(
|
||||||
fn: (argument: Argument) => Promise<Result>,
|
fn: (argument: Argument) => Promise<Result>,
|
||||||
argument: Argument,
|
argument: Argument,
|
||||||
): Promise<Result>;
|
): Promise<Result>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function isRedditWriteOrigin(value: string): boolean {
|
||||||
|
try {
|
||||||
|
return new URL(value).origin === REDDIT_WRITE_ORIGIN;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function createBrowserWriteTransport(page: EvaluatingPage): RedditWriteTransport {
|
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<void> => {
|
||||||
|
if (isRedditWriteOrigin(page.url())) return;
|
||||||
|
await page.goto(REDDIT_WRITE_ORIGIN, { waitUntil: 'domcontentloaded', timeout: 30_000 });
|
||||||
|
};
|
||||||
|
|
||||||
return {
|
return {
|
||||||
// The page already carries the profile's cookies and user agent, so the
|
// The page already carries the profile's cookies and user agent, so the
|
||||||
// request needs no explicit identity of its own.
|
// request needs no explicit identity of its own.
|
||||||
get: (url) => page.evaluate(async ({ target }) => {
|
get: async (url) => {
|
||||||
const response = await fetch(target, {
|
await ensureOrigin();
|
||||||
credentials: 'include',
|
return page.evaluate(async ({ target }) => {
|
||||||
headers: { Accept: 'application/json' },
|
const response = await fetch(target, {
|
||||||
});
|
credentials: 'include',
|
||||||
return { status: response.status, body: await response.text() };
|
headers: { Accept: 'application/json' },
|
||||||
}, { target: url }),
|
});
|
||||||
|
return { status: response.status, body: await response.text() };
|
||||||
|
}, { target: url });
|
||||||
|
},
|
||||||
|
|
||||||
postForm: (url, form, headers) => page.evaluate(async ({ target, fields, extraHeaders }) => {
|
postForm: async (url, form, headers) => {
|
||||||
const body = new URLSearchParams(fields).toString();
|
await ensureOrigin();
|
||||||
const response = await fetch(target, {
|
return page.evaluate(async ({ target, fields, extraHeaders }) => {
|
||||||
method: 'POST',
|
const body = new URLSearchParams(fields).toString();
|
||||||
credentials: 'include',
|
const response = await fetch(target, {
|
||||||
headers: { Accept: 'application/json', ...extraHeaders },
|
method: 'POST',
|
||||||
body,
|
credentials: 'include',
|
||||||
});
|
headers: { Accept: 'application/json', ...extraHeaders },
|
||||||
return { status: response.status, body: await response.text() };
|
body,
|
||||||
}, { target: url, fields: form, extraHeaders: headers ?? {} }),
|
});
|
||||||
|
return { status: response.status, body: await response.text() };
|
||||||
|
}, { target: url, fields: form, extraHeaders: headers ?? {} });
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -149,6 +149,58 @@ describe('RedditCommentClient.postComment', () => {
|
|||||||
expect(transport.posts).toHaveLength(0);
|
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', () => {
|
describe('Reddit error codes', () => {
|
||||||
it('maps RATELIMIT to an account-scoped wait and cools the shared scheduler', async () => {
|
it('maps RATELIMIT to an account-scoped wait and cools the shared scheduler', async () => {
|
||||||
const extendCooldown = vi.fn();
|
const extendCooldown = vi.fn();
|
||||||
@@ -408,9 +460,86 @@ describe('RedditCommentClient.fetchPubliclyVisible', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('createBrowserWriteTransport', () => {
|
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 <Result, Argument>(
|
||||||
|
fn: (argument: Argument) => Promise<Result>,
|
||||||
|
argument: Argument,
|
||||||
|
): Promise<Result> => {
|
||||||
|
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 () => {
|
it('runs both verbs inside the page so requests carry the profile session', async () => {
|
||||||
const calls: unknown[] = [];
|
const calls: unknown[] = [];
|
||||||
const page = {
|
const page = {
|
||||||
|
url: () => 'https://old.reddit.com/',
|
||||||
|
goto: async () => null,
|
||||||
evaluate: async <Result, Argument>(
|
evaluate: async <Result, Argument>(
|
||||||
fn: (argument: Argument) => Promise<Result>,
|
fn: (argument: Argument) => Promise<Result>,
|
||||||
argument: Argument,
|
argument: Argument,
|
||||||
|
|||||||
Reference in new issue
Block a user