Migrate networking library from postman-request to needle#282
Open
TwitchBronBron wants to merge 15 commits into
Open
Migrate networking library from postman-request to needle#282TwitchBronBron wants to merge 15 commits into
postman-request to needle#282TwitchBronBron wants to merge 15 commits into
Conversation
Replace the unmaintained `postman-request` dependency with `needle`. To keep this a non-breaking change, a thin compatibility shim (`src/request.ts`) reconstructs the `request`/`postman-request`-style response shape that roku-deploy exposes on results and thrown errors (`results.response.statusCode`, `results.response.headers`, `results.response.request.host`, string `results.body`). The shim handles three quirks discovered while validating against a real Roku device: needle rejects empty multipart values (request dropped them), needle returns a Buffer body under `parse_response: false` (request returned a string), and needle emits an intermediate 401 `response` event during the streaming digest dance (request only surfaced the final response). It also only enables multipart when form data is actually present, so bodyless POSTs (e.g. ECP keypress) keep working. Adds full real-device test coverage over every networking-enabled RokuDeploy method in `device.spec.ts`, with structure/type-tolerant assertions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e shim The `results`/`response` object that roku-deploy attaches to thrown errors is part of its public API; consumers read specific paths off it (`results.response.statusCode`, `.headers.server`, `.request.host`, string `results.body`). Getting any of these wrong during the needle migration would be a silent breaking change. - Add a `needleClient` seam to request.ts so the shim's HTTP calls are stubbable (mirrors the `httpClient` seam in fetch.ts). - Add request.spec.ts: unit tests for option translation (qs, digest auth, timeouts, headers), formData/multipart handling (drop empty fields, stream -> file form, no multipart for bodyless POSTs), body coercion (Buffer -> string), response reshape (statusCode/headers/request.host/href), error passthrough, and the streaming get path (err->error bridge, intermediate-401 suppression). - Add an "error results structure" block to RokuDeploy.spec.ts that drives RokuDeploy through the REAL shim and pins the results shape for each error type: UnauthorizedDeviceResponseError, InvalidDeviceResponseCodeError, UnparsableDeviceResponseError, FailedDeviceResponseError, EcpNetworkAccessModeDisabledError, UpdateCheckRequiredError, ConnectionResetError. Also hardens buildResponse to pass through a missing response object instead of crashing, preserving checkRequest's `!results.response` guard behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI enforces 100% coverage; the new shim had a few uncovered branches. Add targeted tests for: the username/password auth alias, null/undefined qs values (and an all-null qs leaving the url untouched), null and non-string/non-buffer body coercion, an unparseable url leaving request.host undefined, and a digest-auth 'response' event carrying no response object. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…testing Ran an empirical parity check: installed the last postman-request release (3.17.6) alongside the new needle build and triggered every device-reproducible failure mode against a real Roku, diffing the thrown error structures. 7/8 were already byte-identical. The one real gap: postman-request attached the (string) body to `response.body` in addition to returning it as the callback's 3rd arg, but the shim left `response.body` undefined. A consumer reading `error.results.response.body` would have broken. Fixed buildResponse to set it. (The 8th difference is the raw network error on a dead host -- ETIMEDOUT vs ECONNRESET -- which is the underlying HTTP library's own error passed through untouched; the contract, a bare Error with no results, is identical.) Pin the findings with permanent, device-free unit tests that deep-equal the FULL response/results structure the parity run confirmed (request.spec.ts and the RokuDeploy.spec.ts error-results block). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The widened buildResponse (full IncomingMessage + maximized request object) had two coverage-blocking spots that were over-engineering, not real safeguards: - a try/catch around `url.parse()`, whose catch was unreachable (`url.parse` is total for string input — a bare token yields null host/hostname, never throws), - an acronym special-case in the header title-caser (WWW/TE/DNT/ETag), which only applies to response headers, not the outgoing-request headers this touches. Removed both, and updated the request.spec assertions to match the parity-faithful shape that url.parse produces: response.request.host is the hostname (no port), the port lives on response.request.uri.host/uri.port, unparseable urls yield null (matching postman-request, which also used url.parse), and outgoing headers are title-cased. preversion is green: build + lint + 421 tests passing + 100% coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TwitchBronBron
force-pushed
the
migrate-postman-request-to-needle
branch
from
637dfda to
7ae80ae
Compare
chrisdp
reviewed
Jun 30, 2026
TwitchBronBron
changed the title
TwitchBronBron
changed the title
postman-request to needle
…uest-to-needle # Conflicts: # package-lock.json # package.json
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chrisdp
approved these changes
Jul 2, 2026
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the unmaintained
postman-requestwithneedle.Non-breaking. roku-deploy attaches a
request-shapedresults/responseobject to thrown errors and return values, so a thin shim (src/request.ts) reconstructs that exact shape on top of needle —response.statusCode,response.headers,response.request.host, and a stringbody.Also drops ~860 lines from the lockfile.
How
The shim exposes the slice of the
requestAPI roku-deploy uses —post(opts, cb),get(opts, cb), and the callback-less streamingget(opts).on(...).pipe(...)— and translates it to needle. Digest auth is now native to needle (auth: 'digest'), matching postman-request's challenge/response behavior.Four quirks surfaced while validating against a real Roku, each handled in the shim:
requestsilently dropped empty fields; needle throws"value missing for multipart!". The shim drops them."Empty multipart body"on an empty body (e.g. ECP keypress inpressHomeButton), somultipartis only enabled when form data is actually present.parse_response: falseneedle returns aBuffer;requestreturned a string andcheckRequestguards ontypeof body === 'string'. The shim coerces to a string.401response before retrying withAuthorization;getToFiletreats any non-200 as failure. The shim swallows that one intermediate 401.Testing
needledirectly.src/device.spec.tsexercises every networking-enabledRokuDeploymethod against a real Roku, with type/shape-tolerant assertions so it won't break as device data changes. Signing and disruptive (reboot / update-check) tests are gated behindROKU_SIGNING_PASSWORD/ROKU_RUN_DISRUPTIVE=1.Validated end-to-end against a device:
getDeviceInfo,getDevId,getEcpNetworkAccessMode,validateDeveloperPassword,pressHomeButton,deploy/publish(digest + multipart zip upload),takeScreenshot(POST + stream-to-file),convertToSquashfs,deleteInstalledChannel,deleteComponentLibrary/deleteAllComponentLibraries, and the firmware-gatedrebootDevice/checkForUpdate.🤖 Generated with Claude Code