Skip to content

feat: modernize deps, replace spdy with node:http2, migrate CI to GitHub Actions - #17

Open
FacePlant101 wants to merge 6 commits into
watson:masterfrom
FacePlant101:devin/1782371177-modernize-stage1
Open

feat: modernize deps, replace spdy with node:http2, migrate CI to GitHub Actions#17
FacePlant101 wants to merge 6 commits into
watson:masterfrom
FacePlant101:devin/1782371177-modernize-stage1

Conversation

@FacePlant101

@FacePlant101 FacePlant101 commented Jun 25, 2026

Copy link
Copy Markdown

Summary

The spdy package hard-crashes on Node 22+ (process.binding('http_parser') removed), making the CLI completely broken on modern Node. This PR replaces it with the built-in node:http2 module, modernizes all dependencies to their latest CJS-compatible versions, and removes the stale @juji/cookie-manager dependency entirely.

Key changes

lib/trace.js rewrite — spdynode:http2:

The old code used spdy.createAgent() for all HTTPS requests. Now HTTPS and HTTP use separate code paths:

_follow(url, ms)
  ├── https: → _followH2(parsed, ms)   // node:http2 session + stream
  └── http:  → _followH1(parsed, ms)   // node:http (unchanged behavior)

_followH2 opens an http2.connect(origin) session, sends HEAD via session.request(), and maps the HTTP/2 response:

  • statusCode:status pseudo-header
  • statusMessage'' (HTTP/2 has no reason phrase)
  • httpVersionMajor/Minor2/0
  • headers ← response headers with pseudo-headers stripped

Trace.setProtocol() removed — protocol is known statically per code path.

Other code changes:

  • url.parse()new URL() (avoids DEP0169 deprecation)
  • util.inherits()class Trace extends Stream.Readable
  • @juji/cookie-manager removed — replaced with an inline Cookies class (~35 lines) covering store(), prepare(), and length
  • normalizeUrl() call gains sortQueryParameters: false (v5 sorts by default, v1 did not)
  • Fixed _setCookie bug: was always storing headers['set-cookie'][0] in the loop instead of [i]
  • Fixed redirect handling: resolve relative Location headers against current URL using new URL(location, currentUrl).href

Dependency bumps:

Package Before After
spdy ^3.4.4 removed
cookie-manager / @juji/cookie-manager ^0.0.19 removed (inlined)
chalk ^2.3.0 ^4.1.2
normalize-url ^1.9.0 ^5.3.1
readable-stream ^2.3.3 ^4.7.0
standard ^9.0.2 ^17.1.2
tape ^4.8.0 ^5.10.2

Tests: Replaced broken test URLs (NYT returns 403, Twitter was already skipped) with google.com which reliably returns 200. All tests pass with 0 skips.

CI: Travis CI (.travis.yml) → GitHub Actions (Node 22, 24, 26 matrix).

engines: Added "node": ">=22".

All standard v17 lint warnings fixed (varconst/let, object shorthand).

Link to Devin session: https://app.devin.ai/sessions/31e62f90db4c4e19a7a1eed5302f18e8
Requested by: @avassdal

devin-ai-integration Bot and others added 4 commits June 25, 2026 07:08
…Hub Actions

- Replace spdy (broken on Node 22+) with built-in node:http2 for HTTPS
- Replace cookie-manager with @juji/cookie-manager (drop-in, npm deprecated original)
- Bump chalk ^2.3.0 -> ^4.1.2 (last CJS version)
- Bump normalize-url ^1.9.0 -> ^5.3.1 (last CJS version, add sortQueryParameters: false)
- Bump readable-stream ^2.3.3 -> ^4.7.0
- Bump standard ^9.0.2 -> ^17.1.2, tape ^4.8.0 -> ^5.10.2
- Migrate url.parse() to new URL()
- Migrate util.inherits() to class extends
- Remove Trace.setProtocol() (protocol known statically per code path)
- Add engines: { node: '>=18' }
- Remove .travis.yml, add .github/workflows/ci.yml (Node 18, 20, 22 matrix)
- Fix all standard v17 lint errors (var -> const/let, object shorthand)

Co-Authored-By: Aleksander Vassdal <aleksander.vassdal@gmail.com>
NYT (nyti.ms) now blocks HEAD requests with 403, same as Twitter (t.co)
which was already skipped. This is a server-side change, not a code bug.

Co-Authored-By: Aleksander Vassdal <aleksander.vassdal@gmail.com>
- Fix redirect handling: resolve relative Location headers against
  the current URL using new URL(location, currentUrl).href
- Replace broken test URLs (NYT returns 403, Twitter already skipped)
  with google.com which reliably returns 200 and sets cookies
- All tests now pass with no skips

Co-Authored-By: Aleksander Vassdal <aleksander.vassdal@gmail.com>
Node 18 and 20 are EOL/deprecated. Update CI matrix to current
supported versions and bump engines accordingly.

Co-Authored-By: Aleksander Vassdal <aleksander.vassdal@gmail.com>
@socket-security

socket-security Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​chalk@​4.1.210010010082100
Addednpm/​readable-stream@​4.7.09910010083100
Addednpm/​normalize-url@​5.3.110010010085100
Addednpm/​standard@​17.1.2991009986100
Addednpm/​tape@​5.10.29810010093100

View full report

@socket-security

socket-security Bot commented Jun 25, 2026

Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm es-abstract is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: package-lock.jsonnpm/tape@5.10.2npm/standard@17.1.2npm/es-abstract@1.24.2

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/es-abstract@1.24.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm eslint-plugin-react is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: package-lock.jsonnpm/standard@17.1.2npm/eslint-plugin-react@7.37.5

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/eslint-plugin-react@7.37.5. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm js-yaml is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: package-lock.jsonnpm/standard@17.1.2npm/js-yaml@4.2.0

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/js-yaml@4.2.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

FacePlant101 and others added 2 commits June 25, 2026 09:37
Remove the stale third-party dependency and inline a minimal Cookies
class (~35 lines) that covers the same surface: store(), prepare(), and
length. No behaviour change; all 14 tests pass.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@FacePlant101 FacePlant101 mentioned this pull request Jun 25, 2026
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.

1 participant