Skip to content

Commit b7d1df9

Browse files
authored
refactor!: drop the underscore prefix from non-public members (#1059)
Non-public members in this repo carried a leading underscore on top of the `private` / `protected` keyword. Crawlee v4 dropped that convention and we agreed to follow it here. This renames every such member across `src/`, so none are left. `LoggerActorRedirect._log()` and `_outputWithConsole()` in `src/resource_clients/log.ts` keep their names, since both come from the `Logger` base class in `@apify/log`. The code samples in `CONTRIBUTING.md` and the public API report follow the rename. ## Renamed helpers The protected helpers on the base clients show up in the API report, so subclasses calling the old names have to switch. Where a bare name would collide with a public method, the helper got a suffix. | Old | New | | --- | --- | | `_url` | `buildUrl` | | `_publicUrl` | `buildPublicUrl` | | `_params` | `buildParams` | | `_subResourceOptions` | `subResourceOptions` | | `_toSafeId` | `toSafeId` | | `_listPaginatedFromCallback` | `listPaginatedFromCallback` | | `_get` | `getResource` | | `_update` | `updateResource` | | `_delete` | `deleteResource` | | `_waitForFinish` | `waitForJobFinish` | | `_list` | `listResources` | | `_listPaginated` | `listResourcesPaginated` | | `_create` | `createResource` | | `_getOrCreate` | `getOrCreateResource` | | `_batchAddRequests` | `addRequestBatch` | | `_batchAddRequestsWithRetries` | `addRequestBatchWithRetries` | The names match what #1046 picked for the files it touches, so whichever of the two lands second rebases onto near-identical lines. ## ApifyApiError.clientMethod `clientMethod` scrapes the stack with `\._?([A-Za-z]+)`, and the underscore was what turned an internal frame like `ActorCollectionClient._list` into the public-looking `list`. A public method that returns the promise of a shared helper without awaiting it leaves only the helper's frame on the async stack, so after the rename the field would report `listResources` to users. A `PUBLIC_METHOD_BY_HELPER` map in `apify_api_error.ts` maps helper frames back to the public method, and `test/apify_api_error.test.ts` covers both the Node and the browser build. Closes #1057 *✍️ Drafted by Claude Code*
1 parent b78c8bc commit b7d1df9

11 files changed

Lines changed: 194 additions & 176 deletions

File tree

CONTRIBUTING.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ This project uses [oxlint](https://oxc.rs/docs/guide/usage/linter) and [oxfmt](h
159159
- Use single quotes for strings
160160
- Add trailing commas in multiline structures
161161
- Export types and interfaces alongside implementations
162+
- Declare private members as `#private` identifiers, and use `protected` only for helpers that subclasses call
162163
- Avoid `any` types where possible (though the oxlint rule is disabled)
163164

164165
### Before Committing
@@ -306,6 +307,7 @@ When adding support for a new API resource:
306307
// src/resource_clients/my_resource.ts
307308
import { ApiClientSubResourceOptions } from '../base/api_client';
308309
import { ResourceClient } from '../base/resource_client';
310+
import * as schemas from '../schemas';
309311

310312
export class MyResourceClient extends ResourceClient {
311313
constructor(options: ApiClientSubResourceOptions) {
@@ -316,15 +318,15 @@ export class MyResourceClient extends ResourceClient {
316318
}
317319

318320
async get(): Promise<MyResource | undefined> {
319-
return this._get();
321+
return this.getResource(schemas.MyResource());
320322
}
321323

322324
async update(newFields: MyResourceUpdate): Promise<MyResource> {
323-
return this._update(newFields);
325+
return this.updateResource(schemas.MyResource(), newFields);
324326
}
325327

326328
async delete(): Promise<void> {
327-
return this._delete();
329+
return this.deleteResource();
328330
}
329331
}
330332
```
@@ -335,6 +337,7 @@ export class MyResourceClient extends ResourceClient {
335337
// src/resource_clients/my_resource_collection.ts
336338
import { ApiClientSubResourceOptions } from '../base/api_client';
337339
import { ResourceCollectionClient } from '../base/resource_collection_client';
340+
import * as schemas from '../schemas';
338341
import { PaginatedList } from '../utils';
339342

340343
export class MyResourceCollectionClient extends ResourceCollectionClient {
@@ -346,11 +349,11 @@ export class MyResourceCollectionClient extends ResourceCollectionClient {
346349
}
347350

348351
async list(options?: MyResourceListOptions): Promise<PaginatedList<MyResource>> {
349-
return this._list(options);
352+
return this.listResources(schemas.ListOfMyResources(), options);
350353
}
351354

352355
async create(resource: MyResourceCreate): Promise<MyResource> {
353-
return this._create(resource);
356+
return this.createResource(schemas.MyResource(), resource);
354357
}
355358
}
356359
```
@@ -360,11 +363,11 @@ export class MyResourceCollectionClient extends ResourceCollectionClient {
360363
```typescript
361364
// In src/apify_client.ts
362365
myResource(id: string): MyResourceClient {
363-
return new MyResourceClient(this._subResourceOptions({ id }));
366+
return new MyResourceClient({ id, ...this.subClientOptions() });
364367
}
365368

366369
myResources(): MyResourceCollectionClient {
367-
return new MyResourceCollectionClient(this._options());
370+
return new MyResourceCollectionClient(this.subClientOptions());
368371
}
369372
```
370373

docs/04_upgrading/upgrading_v3.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,3 +368,36 @@ export HTTPS_PROXY=http://proxy.example.com:3128
368368
```
369369

370370
The same `proxy-agent` upgrade removes the `[DEP0169] DeprecationWarning` about `url.parse()` that Node.js 24 and newer printed on the client's first request.
371+
372+
## Non-public members no longer carry an underscore
373+
374+
Members declared `private` or `protected` had a leading underscore on top of the keyword in v2, and the underscore is gone in v3. Code that calls only the public methods of a client is unaffected. A class that extends one of the client classes and calls a protected helper has to switch to the new name. The base classes below are not exported, so you reach these members by extending a concrete client such as `ActorClient`.
375+
376+
| Class | v2 | v3 |
377+
| --- | --- | --- |
378+
| `ApiClient` | `_url()` | `buildUrl()` |
379+
| `ApiClient` | `_publicUrl()` | `buildPublicUrl()` |
380+
| `ApiClient` | `_params()` | `buildParams()` |
381+
| `ApiClient` | `_subResourceOptions()` | `subResourceOptions()` |
382+
| `ApiClient` | `_toSafeId()` | `toSafeId()` |
383+
| `ApiClient` | `_listPaginatedFromCallback()` | `listPaginatedFromCallback()` |
384+
| `ResourceClient` | `_get()` | `getResource()` |
385+
| `ResourceClient` | `_update()` | `updateResource()` |
386+
| `ResourceClient` | `_delete()` | `deleteResource()` |
387+
| `ResourceClient` | `_waitForFinish()` | `waitForJobFinish()` |
388+
| `ResourceCollectionClient` | `_list()` | `listResources()` |
389+
| `ResourceCollectionClient` | `_listPaginated()` | `listResourcesPaginated()` |
390+
| `ResourceCollectionClient` | `_create()` | `createResource()` |
391+
| `ResourceCollectionClient` | `_getOrCreate()` | `getOrCreateResource()` |
392+
| `RequestQueueClient` | `_batchAddRequests()` | `addRequestBatch()` |
393+
| `RequestQueueClient` | `_batchAddRequestsWithRetries()` | `addRequestBatchWithRetries()` |
394+
395+
A helper got a suffix wherever the bare name would collide with a public method of the same class, which is why `_get()` is now `getResource()` and not `get()`.
396+
397+
<ApiLink to="class/LoggerActorRedirect">`LoggerActorRedirect`</ApiLink> keeps `_log()`, since it overrides the method of that name on the `Logger` base class in `@apify/log`.
398+
399+
The `clientMethod` field on <ApiLink to="class/ApifyApiError">`ApifyApiError`</ApiLink> is parsed from the stack trace, and a public method that delegates to one of these helpers is reported under the helper's name. An error from `client.actor(id).get()` says `ActorClient.getResource`, where v2 said `ActorClient.get`. Adjust anything that matches on these values in your logs.
400+
401+
### Private members are private at runtime
402+
403+
Members that v2 declared `private` are declared with a `#` in v3, so the JavaScript runtime enforces the boundary, where v2 relied on the type checker. Code that reached one of them through a cast, such as `(client.httpClient as any).nodeInitPromise`, throws a `TypeError` in v3. They also don't appear when you spread an instance, iterate `Object.keys()` on it, or pass it to `JSON.stringify()`. The `protected` helpers in the table keep the `protected` keyword, so a subclass can call them.

src/apify_api_error.ts

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,6 @@ export type { ApifyApiErrorType } from './models.js';
1818
*/
1919
const CLIENT_METHOD_REGEX = /at( async)? ([A-Za-z]+(Collection)?Client)\.([A-Za-z]+) \(/;
2020

21-
/**
22-
* A public method that returns the promise of a shared helper without awaiting it leaves only the helper on
23-
* the stack, so the helper is reported under the method the caller invoked.
24-
* @private
25-
*/
26-
const PUBLIC_METHOD_BY_HELPER: Record<string, string> = {
27-
getResource: 'get',
28-
updateResource: 'update',
29-
deleteResource: 'delete',
30-
waitForJobFinish: 'waitForFinish',
31-
listResources: 'list',
32-
createResource: 'create',
33-
getOrCreateResource: 'getOrCreate',
34-
addRequestBatch: 'batchAddRequests',
35-
addRequestBatchWithRetries: 'batchAddRequests',
36-
};
37-
3821
/**
3922
* An `ApifyApiError` is thrown for successful HTTP requests that reach the API,
4023
* but the API responds with an error response. Typically, those are rate limit
@@ -53,8 +36,9 @@ export class ApifyApiError extends Error {
5336
override name: string;
5437

5538
/**
56-
* The invoked resource client and the method. Known issue: Sometimes it displays
57-
* as `unknown` because it can't be parsed from a stack trace.
39+
* The resource client and the method parsed from the stack trace, such as `ActorClient.getResource`. A public
40+
* method that delegates to a shared helper of its class is reported under the helper. The value is `unknown`
41+
* when the stack trace cannot be parsed.
5842
*/
5943
clientMethod: string;
6044

@@ -131,17 +115,17 @@ export class ApifyApiError extends Error {
131115
super(message);
132116

133117
this.name = this.constructor.name;
134-
this.clientMethod = this._extractClientAndMethodFromStack();
118+
this.clientMethod = this.#extractClientAndMethodFromStack();
135119
this.statusCode = response.status;
136120
this.type = type;
137121
this.attempt = attempt;
138122
this.httpMethod = response.config?.method;
139-
this.path = this._safelyParsePathFromResponse(response);
123+
this.path = this.#safelyParsePathFromResponse(response);
140124

141125
const stack = this.stack!;
142126

143127
this.originalStack = stack.slice(stack.indexOf('\n'));
144-
this.stack = this._createApiStack();
128+
this.stack = this.#createApiStack();
145129

146130
this.data = errorData;
147131
}
@@ -156,7 +140,7 @@ export class ApifyApiError extends Error {
156140
return new ErrorClass(response, attempt);
157141
}
158142

159-
private _safelyParsePathFromResponse(response: AxiosResponse) {
143+
#safelyParsePathFromResponse(response: AxiosResponse) {
160144
const urlString = response.config?.url;
161145
let url;
162146
try {
@@ -167,10 +151,10 @@ export class ApifyApiError extends Error {
167151
return url.pathname + url.search;
168152
}
169153

170-
private _extractClientAndMethodFromStack() {
154+
#extractClientAndMethodFromStack() {
171155
const match = this.stack!.match(CLIENT_METHOD_REGEX);
172156
if (!match) return 'unknown';
173-
return `${match[2]}.${PUBLIC_METHOD_BY_HELPER[match[4]] ?? match[4]}`;
157+
return `${match[2]}.${match[4]}`;
174158
}
175159

176160
/**
@@ -187,7 +171,7 @@ export class ApifyApiError extends Error {
187171
* httpMethod: post
188172
* path: /v2/actor-tasks/user~my-task/runs
189173
*/
190-
private _createApiStack() {
174+
#createApiStack() {
191175
const { name, ...props } = this;
192176

193177
const stack = Object.entries(props)

0 commit comments

Comments
 (0)