fix: 🐞Update postForId to use correct Substack API endpoint - #89
Conversation
…id/{id})
Co-authored-by: jakub-k-slys <29287862+jakub-k-slys@users.noreply.github.com>
getPostById Uses Incorrect Endpoint
jakub-k-slys
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #89 +/- ##
==========================================
+ Coverage 92.62% 92.68% +0.05%
==========================================
Files 9 9
Lines 244 246 +2
Branches 43 43
==========================================
+ Hits 226 228 +2
Misses 1 1
Partials 17 17
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the postForId method to call the correct Substack API endpoint, adds error handling for missing posts, and updates unit tests to cover both success and error cases.
- Updated the endpoint URL in
SubstackClient.postForId() - Wrapped the HTTP call in a try/catch with a descriptive error message
- Adjusted unit tests to assert the new endpoint and error behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/unit/substack-client.test.ts | Updated expected URL in test and added an error scenario test |
| src/substack-client.ts | Changed endpoint path and added error handling logic |
| const post = await this.httpClient.get<SubstackPost>(`/api/v1/posts/by-id/${id}`) | ||
| return new Post(post, this.httpClient) | ||
| } catch (error) { | ||
| throw new Error(`Post with ID ${id} not found: ${(error as Error).message}`) |
There was a problem hiding this comment.
Catching all errors unconditionally may mask non-404 issues (e.g., network or permission errors). Consider inspecting the error (e.g., checking error.response?.status) and rethrowing unexpected errors so only genuine "not found" cases are handled here.
| throw new Error(`Post with ID ${id} not found: ${(error as Error).message}`) | |
| if (error.response?.status === 404) { | |
| throw new Error(`Post with ID ${id} not found`); | |
| } | |
| throw error; // Rethrow the original error for non-404 cases |
| const post = await this.httpClient.get<SubstackPost>(`/api/v1/posts/by-id/${id}`) | ||
| return new Post(post, this.httpClient) | ||
| } catch (error) { | ||
| throw new Error(`Post with ID ${id} not found: ${(error as Error).message}`) |
There was a problem hiding this comment.
Throwing a new Error here discards the original stack trace. Consider using the cause option (Node.js >=16.9) like throw new Error(msg, { cause: error as Error }) to preserve the original error details.
| throw new Error(`Post with ID ${id} not found: ${(error as Error).message}`) | |
| throw new Error(`Post with ID ${id} not found: ${(error as Error).message}`, { cause: error as Error }) |
|
🎉 This PR is included in version 0.15.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Problem
The
postForId(postId)method was using an incorrect Substack API endpoint, causing requests to fail with 404 errors or return invalid responses.Current (incorrect) endpoint:
/api/v1/posts/{id}Correct endpoint:
/api/v1/posts/by-id/{id}Changes
SubstackClient.postForId()methodExample
Verification
Fixes #88.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
test.substack.com/usr/local/bin/node /home/REDACTED/work/substack-api/substack-api/node_modules/jest-worker/build/processChild.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.