[pull] master from apify:master

package.json

@@ -101,7 +101,7 @@
         "got": "^13.0.0",
         "husky": "^9.0.11",
         "is-ci": "^4.0.0",
-        "lerna": "^9.0.7",
+        "lerna": "^10.0.0",
         "lint-staged": "^17.0.0",
         "nock": "^13.4.0",
         "playwright": "1.62.0",

packages/impit-client/src/index.ts

@@ -2,7 +2,14 @@ import { pipeline, Readable, Transform } from 'node:stream';
 import { type ReadableStream } from 'node:stream/web';
 import { isGeneratorObject } from 'node:util/types';
 
-import type { BaseHttpClient, HttpRequest, HttpResponse, ResponseTypes, StreamingHttpResponse } from '@crawlee/core';
+import type {
+    BaseHttpClient,
+    HttpRequest,
+    HttpResponse,
+    RedirectHandler,
+    ResponseTypes,
+    StreamingHttpResponse,
+} from '@crawlee/core';
 import type { HttpMethod, ImpitOptions, ImpitResponse, RequestInit } from 'impit';
 import { Impit } from 'impit';
 import type { CookieJar as ToughCookieJar } from 'tough-cookie';
@@ -19,6 +26,8 @@ interface ResponseWithRedirects {
     redirectUrls: URL[];
 }
 
+type SimpleHeaders = Record<string, string | string[] | undefined>;
+
 /**
  * A HTTP client implementation based on the `impit library.
  */
@@ -119,6 +128,27 @@ export class ImpitHttpClient implements BaseHttpClient {
         return false;
     }
 
+    /**
+     * Converts Fetch/Impit headers into a simple header map.
+     * `Object.fromEntries` would keep only the last `set-cookie` value, so those are collected separately.
+     */
+    private intoSimpleHeaders(headers: Headers): SimpleHeaders {
+        const result: SimpleHeaders = {};
+
+        for (const [key, value] of headers.entries()) {
+            if (key === 'set-cookie') continue;
+            result[key] = value;
+        }
+
+        const setCookies = headers.getSetCookie();
+
+        if (setCookies.length > 0) {
+            result['set-cookie'] = setCookies.length === 1 ? setCookies[0] : setCookies;
+        }
+
+        return result;
+    }
+
     /**
      * Common implementation for `sendRequest` and `stream` methods.
      * @param request `HttpRequest` object
@@ -130,6 +160,7 @@ export class ImpitHttpClient implements BaseHttpClient {
             redirectCount?: number;
             redirectUrls?: URL[];
         },
+        onRedirect?: RedirectHandler,
     ): Promise<ResponseWithRedirects> {
         if ((redirects?.redirectCount ?? 0) > this.maxRedirects) {
             throw new Error(`Too many redirects, maximum is ${this.maxRedirects}.`);
@@ -159,16 +190,44 @@ export class ImpitHttpClient implements BaseHttpClient {
                 throw new Error('Redirect response missing location header.');
             }
 
+            const nextRedirectUrls = [...(redirects?.redirectUrls ?? []), redirectUrl];
+            const updatedRequest: { url?: string | URL; headers: SimpleHeaders } = {
+                url: redirectUrl.href,
+                headers: { ...(request.headers ?? {}) },
+            };
+
+            // Match GotScrapingHttpClient: allow HttpCrawler to persist redirect cookies into the session
+            // and mutate Cookie / URL for the next hop.
+            onRedirect?.(
+                {
+                    redirectUrls: nextRedirectUrls,
+                    url,
+                    statusCode: response.status,
+                    statusMessage: response.statusText,
+                    headers: this.intoSimpleHeaders(response.headers),
+                    trailers: {},
+                    complete: true,
+                },
+                updatedRequest,
+            );
+
+            const nextUrl =
+                typeof updatedRequest.url === 'string'
+                    ? updatedRequest.url
+                    : (updatedRequest.url?.href ?? redirectUrl.href);
+
             return this.getResponse(
                 {
                     ...request,
                     method: this.shouldRewriteRedirectToGet(response.status, request.method) ? 'GET' : request.method,
-                    url: redirectUrl.href,
+                    url: nextUrl,
+                    headers: updatedRequest.headers,
                 },
                 {
                     redirectCount: (redirects?.redirectCount ?? 0) + 1,
-                    redirectUrls: [...(redirects?.redirectUrls ?? []), redirectUrl],
+                    redirectUrls: nextRedirectUrls,
                 },
+                onRedirect,
             );
         }
 
@@ -203,7 +262,7 @@ export class ImpitHttpClient implements BaseHttpClient {
         }
 
         return {
-            headers: Object.fromEntries(response.headers.entries()),
+            headers: this.intoSimpleHeaders(response.headers),
             statusCode: response.status,
             url: response.url,
             request,
@@ -243,8 +302,8 @@ export class ImpitHttpClient implements BaseHttpClient {
     /**
      * @inheritDoc
      */
-    async stream(request: HttpRequest): Promise<StreamingHttpResponse> {
-        const { response, redirectUrls } = await this.getResponse(request);
+    async stream(request: HttpRequest, onRedirect?: RedirectHandler): Promise<StreamingHttpResponse> {
+        const { response, redirectUrls } = await this.getResponse(request, undefined, onRedirect);
         const [stream, getDownloadProgress] = this.getStreamWithProgress(response);
 
         return {
@@ -258,7 +317,7 @@ export class ImpitHttpClient implements BaseHttpClient {
             },
             uploadProgress: { percent: 100, transferred: 0 },
             redirectUrls,
-            headers: Object.fromEntries(response.headers.entries()),
+            headers: this.intoSimpleHeaders(response.headers),
             trailers: {},
         };
     }

test/core/crawlers/http_crawler.test.ts

@@ -39,6 +39,12 @@ router.set('/cookies', (req, res) => {
     res.end(JSON.stringify(req.headers.cookie));
 });
 
+router.set('/setCookie', (req, res) => {
+    res.setHeader('content-type', 'text/html');
+    res.setHeader('set-cookie', 'first=1');
+    res.end();
+});
+
 router.set('/redirectWithoutCookies', (req, res) => {
     res.setHeader('location', '/cookies');
     res.statusCode = 302;
@@ -238,6 +244,27 @@ describe.each(
         expect(results).toStrictEqual(['foo=bar']);
     });
 
+    test('handles cookies from redirects when the session already has cookies', async () => {
+        const results: string[] = [];
+
+        const crawler = new HttpCrawler({
+            httpClient,
+            sessionPoolOptions: {
+                maxPoolSize: 1,
+                // isolated so that cookies stored by the other tests / clients don't leak in
+                persistStateKey: `SDK_SESSION_POOL_STATE_${httpClient.constructor.name}`,
+            },
+            maxConcurrency: 1,
+            requestHandler: async ({ body }) => {
+                results.push(body.toString());
+            },
+        });
+
+        await crawler.run([`${url}/setCookie`, `${url}/redirectAndCookies`]);
+
+        expect(results[1]).toBe('"first=1; foo=bar"');
+    });
+
     test('handles cookies from redirects - no empty cookie header', async () => {
         const results: string[] = [];
 

test/core/impit_http_client.test.ts

@@ -9,6 +9,37 @@ vi.mock('impit', () => ({
     ),
 }));
 
+function createRedirectResponse(status: number, location: string, setCookie: string[] = []) {
+    const headers = new Headers({ location });
+    for (const cookie of setCookie) headers.append('set-cookie', cookie);
+
+    return {
+        status,
+        statusText: 'Found',
+        url: 'http://example.com/start',
+        headers,
+        body: undefined,
+    };
+}
+
+function createFinalResponse(body = 'ok') {
+    return {
+        status: 200,
+        statusText: 'OK',
+        url: 'http://example.com/final',
+        headers: new Headers({ 'content-type': 'text/plain', 'content-length': String(body.length) }),
+        body: new ReadableStream({
+            start(controller) {
+                controller.enqueue(new TextEncoder().encode(body));
+                controller.close();
+            },
+        }),
+        text: async () => body,
+        json: async () => ({ body }),
+        bytes: async () => Buffer.from(body),
+    };
+}
+
 describe('ImpitHttpClient', () => {
     beforeEach(() => {
         vi.mocked(Impit).mockClear();
@@ -31,4 +62,70 @@ describe('ImpitHttpClient', () => {
 
         expect(Impit).toHaveBeenCalledTimes(2);
     });
+
+    test('stream() invokes onRedirect and forwards mutated Cookie header to the next hop', async () => {
+        const httpClient = new ImpitHttpClient({ cacheClients: false });
+        const fetchMock = vi.fn();
+
+        vi.mocked(Impit).mockImplementation(
+            class {
+                fetch = fetchMock;
+            } as any,
+        );
+
+        fetchMock
+            .mockResolvedValueOnce(createRedirectResponse(302, '/final', ['session=abc', 'other=def']))
+            .mockResolvedValueOnce(createFinalResponse('done'));
+
+        const onRedirect = vi.fn((_redirectResponse, updatedRequest) => {
+            updatedRequest.headers.Cookie = 'session=abc';
+        });
+
+        const response = await httpClient.stream(
+            {
+                url: 'http://example.com/start',
+                method: 'GET',
+                headers: {},
+            },
+            onRedirect,
+        );
+
+        expect(onRedirect).toHaveBeenCalledTimes(1);
+        expect(fetchMock).toHaveBeenCalledTimes(2);
+
+        const [redirectResponse] = onRedirect.mock.calls[0];
+        expect(redirectResponse.statusCode).toBe(302);
+        expect(redirectResponse.headers['set-cookie']).toEqual(['session=abc', 'other=def']);
+
+        const secondCallHeaders = fetchMock.mock.calls[1][1].headers as Headers;
+        expect(secondCallHeaders.get('Cookie')).toBe('session=abc');
+        expect(fetchMock.mock.calls[1][0]).toBe('http://example.com/final');
+        expect(response.statusCode).toBe(200);
+        expect(response.redirectUrls).toEqual([new URL('http://example.com/final')]);
+    });
+
+    test('stream() follows redirects without onRedirect for API compatibility', async () => {
+        const httpClient = new ImpitHttpClient({ cacheClients: false });
+        const fetchMock = vi.fn();
+
+        vi.mocked(Impit).mockImplementation(
+            class {
+                fetch = fetchMock;
+            } as any,
+        );
+
+        fetchMock
+            .mockResolvedValueOnce(createRedirectResponse(302, 'http://example.com/final'))
+            .mockResolvedValueOnce(createFinalResponse('done'));
+
+        const response = await httpClient.stream({
+            url: 'http://example.com/start',
+            method: 'GET',
+            headers: {},
+        });
+
+        expect(fetchMock).toHaveBeenCalledTimes(2);
+        expect(response.statusCode).toBe(200);
+        expect(response.redirectUrls).toHaveLength(1);
+    });
 });

test/e2e/adaptive-playwright-robots-file/actor/main.js

@@ -1,6 +1,48 @@
+import http from 'node:http';
+
 import { AdaptivePlaywrightCrawler } from '@crawlee/playwright';
 import { Actor } from 'apify';
 
+// Self-contained fixture: robots.txt disallows /cart and /checkout, and the
+// start page links to /cart alongside the allowed /collections/* pages. The
+// crawler may only reach the collections.
+const pages = {
+    '/robots.txt': ['User-agent: *', 'Disallow: /cart', 'Disallow: /checkout', ''].join('\n'),
+    '/': `<!doctype html>
+<html><head><title>Store</title></head>
+<body>
+    <a href="/cart">Cart</a>
+    <a href="/collections/audio">Audio</a>
+    <a href="/collections/tv">TV</a>
+</body></html>`,
+    // Every collection links back to /cart, so the robots.txt rule is what keeps
+    // it out of the dataset rather than a shortage of links to follow.
+    '/collections/audio': `<!doctype html>
+<html><head><title>Audio</title></head>
+<body><a href="/cart">Cart</a> <a href="/">Home</a></body></html>`,
+    '/collections/tv': `<!doctype html>
+<html><head><title>TV</title></head>
+<body><a href="/cart">Cart</a> <a href="/">Home</a></body></html>`,
+    '/cart': '<!doctype html><html><head><title>Cart</title></head><body>Cart</body></html>',
+    '/checkout': '<!doctype html><html><head><title>Checkout</title></head><body>Checkout</body></html>',
+};
+
+const server = http.createServer((req, res) => {
+    const body = pages[req.url];
+    if (body === undefined) {
+        res.statusCode = 404;
+        res.end('Not Found');
+        return;
+    }
+    const type = req.url === '/robots.txt' ? 'text/plain' : 'text/html';
+    res.writeHead(200, { 'content-type': `${type}; charset=utf-8` });
+    res.end(body);
+});
+
+await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve));
+const { port } = server.address();
+const baseUrl = `http://127.0.0.1:${port}`;
+
 await Actor.init({
     storage:
         process.env.STORAGE_IMPLEMENTATION === 'LOCAL'
@@ -9,8 +51,6 @@ await Actor.init({
 });
 
 const crawler = new AdaptivePlaywrightCrawler({
-    // The store rate-limits the platform's shared egress IP, so crawl through a proxy.
-    proxyConfiguration: await Actor.createProxyConfiguration(),
     maxRequestsPerCrawl: 10,
     respectRobotsTxtFile: true,
     onSkippedRequest: (args) => crawler.log.warningOnce(`Request ${args.url} was skipped, reason: ${args.reason}`),
@@ -26,11 +66,13 @@ crawler.router.addDefaultHandler(async ({ log, request, enqueueLinks, pushData }
 });
 
 await crawler.run([
-    'https://warehouse-theme-metal.myshopify.com',
-    'https://warehouse-theme-metal.myshopify.com/checkout', // '/checkout' is disallowed by robots.txt
+    baseUrl,
+    `${baseUrl}/checkout`, // '/checkout' is disallowed by robots.txt
 ]);
 
 const data = await crawler.getData();
 console.table(data.items);
 
+server.close();
+
 await Actor.exit({ exit: Actor.isAtHome() });

test/e2e/adaptive-playwright-robots-file/test.mjs

@@ -3,12 +3,12 @@ import { initialize, getActorTestDir, runActor, expect } from '../tools.mjs';
 const testActorDirname = getActorTestDir(import.meta.url);
 await initialize(testActorDirname);
 
-const { datasetItems } = await runActor(testActorDirname, 16384);
+const { stats, datasetItems } = await runActor(testActorDirname, 16384);
 
-const cartRequest = datasetItems.find((item) => item.url === 'https://warehouse-theme-metal.myshopify.com/cart');
-const checkoutRequest = datasetItems.find(
-    (item) => item.url === 'https://warehouse-theme-metal.myshopify.com/checkout',
-);
+// Without this the two assertions below hold vacuously when the crawl never starts.
+await expect(stats.requestsFinished >= 1, 'All requests finished');
 
-await expect(!cartRequest, '/cart URL is not processed');
-await expect(!checkoutRequest, '/checkout URL is not processed');
+const paths = datasetItems.map((item) => new URL(item.url).pathname);
+
+await expect(!paths.includes('/cart'), '/cart URL is not processed');
+await expect(!paths.includes('/checkout'), '/checkout URL is not processed');