Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable HAR Recordings #180

Closed
wants to merge 3 commits into from
Closed

Enable HAR Recordings #180

wants to merge 3 commits into from

Conversation

FHantke
Copy link
Contributor

@FHantke FHantke commented Dec 8, 2024

Add the functionality to record HAR files (mentioned in #68) without (--har) and with response bodies (plus --har-body).

This commit adds the chrome-har dependency (MIT license).

Also, I'm not a big TS developer so it might be that typing could be better. Happy for feedback.

Cheers, Florian

@FHantke FHantke requested a review from pes10k as a code owner December 8, 2024 17:05
@pes10k
Copy link
Collaborator

pes10k commented Dec 15, 2024

@FHantke thanks for this! I'm reviewing it now, but it doesn't build for me. Before i dig in further, can you confirm that npm run build works for you (and if so, let me know what version of node you're using)?

@pes10k
Copy link
Collaborator

pes10k commented Dec 15, 2024

fwiw, here is what i get

> npx tsc

node_modules/@types/node/globals.d.ts:6:76 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

6 type _Request = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Request;
                                                                             ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:7:77 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

7 type _Response = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Response;
                                                                              ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:8:77 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

8 type _FormData = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").FormData;
                                                                              ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:9:76 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

9 type _Headers = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Headers;
                                                                             ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:11:14 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

11     : import("undici-types").RequestInit;
                ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:13:14 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

13     : import("undici-types").ResponseInit;
                ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:380:25 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

380         : typeof import("undici-types").Request;
                            ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:389:25 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

389         : typeof import("undici-types").Response;
                            ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:396:25 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

396         : typeof import("undici-types").FormData;
                            ~~~~~~~~~~~~~~

node_modules/@types/node/globals.d.ts:403:25 - error TS2792: Cannot find module 'undici-types'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

403         : typeof import("undici-types").Headers;
                            ~~~~~~~~~~~~~~

src/brave/crawl.ts:14:31 - error TS2792: Cannot find module 'devtools-protocol'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

14 import type { Protocol } from 'devtools-protocol';
                                 ~~~~~~~~~~~~~~~~~~~

src/brave/crawl.ts:141:50 - error TS2339: Property 'requestId' does not exist on type 'ExtendedResponseReceivedEvent'.

141         const requestId = responseReceivedParams.requestId;
                                                     ~~~~~~~~~

src/brave/files.ts:4:24 - error TS2792: Cannot find module 'fs-extra'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

4 import { remove } from 'fs-extra'
                         ~~~~~~~~~~

src/brave/puppeteer.ts:3:24 - error TS2792: Cannot find module 'fs-extra'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

3 import fsExtraLib from 'fs-extra'
                         ~~~~~~~~~~

src/declarations.d.ts:66:18 - error TS2304: Cannot find name 'LaunchOptions'.

66   launchOptions: LaunchOptions
                    ~~~~~~~~~~~~~

src/declarations.d.ts:72:35 - error TS2304: Cannot find name 'LaunchOptionsType'.

72 type RunPuppeteerFunc = (options: LaunchOptionsType) => Promise<ProcessType>
                                     ~~~~~~~~~~~~~~~~~

src/declarations.d.ts:72:65 - error TS2304: Cannot find name 'ProcessType'.

72 type RunPuppeteerFunc = (options: LaunchOptionsType) => Promise<ProcessType>
                                                                   ~~~~~~~~~~~

tsconfig.json:2:16 - error TS6053: File '@tsconfig/node20/tsconfig.json' not found.

2     "extends": "@tsconfig/node20/tsconfig.json",
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 18 errors in 6 files.

Errors  Files
    10  node_modules/@types/node/globals.d.ts:6
     2  src/brave/crawl.ts:14
     1  src/brave/files.ts:4
     1  src/brave/puppeteer.ts:3
     3  src/declarations.d.ts:66
     1  tsconfig.json:2```

@FHantke
Copy link
Contributor Author

FHantke commented Dec 16, 2024

Hi @pes10k, yes, the branch builds for me.

$ npm run build

> pagegraph-crawl@1.1.2 build
> npx tsc

(node:94846) ExperimentalWarning: CommonJS module /opt/homebrew/lib/node_modules/npm/node_modules/debug/src/node.js is loading ES Module /opt/homebrew/lib/node_modules/npm/node_modules/supports-color/index.js using require().
Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

I'm building it with node v23.1.0 on MacOS v15.1.1.

@FHantke
Copy link
Contributor Author

FHantke commented Dec 16, 2024

From your output, it seems that node is missing some dependencies such as undici-types. Most of the dependencies and mentioned lines in the code are already part of the pagegraph-crawl main version and nothing I added with my commit. Only 14 import type { Protocol } from 'devtools-protocol'; was added by me. Did you check if you can install the current main version without problems?

@pes10k
Copy link
Collaborator

pes10k commented Dec 16, 2024

@FHantke okie, i think this is just about there. I pushed a commit that adds the following:

  • a lot of linter / formatting fixes (please make sure npm run lint is clean before pushing commits)
  • some clean up in the tests runner
  • removing another dependency (thats not specifically related to this change, but fell out of it neatly)

I'd like there to be a test or two in place for the new functionality before we merge this in and stamp it as 1.1.3. If you have the time today, could you add tests for the --har-body and --har options? Otherwise, if you dont have time or if you'd rather i take a go at it, let me know and i can get to it tonight or tomorrow

In the meantime though, if you can confirm that the branch still works as expected, even w/ my commit, that'd be great

@pes10k pes10k closed this Dec 16, 2024
@pes10k pes10k reopened this Dec 16, 2024
@thypon thypon mentioned this pull request Dec 16, 2024
@thypon
Copy link
Member

thypon commented Dec 16, 2024

Closing this one, in favor of #181 to run internal security checks

@thypon thypon closed this Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants