Skip to content

Check error codes against the ably-common registry at compile time #2281

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.

Sign up for GitHub

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

Open
lmars wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Check error codes against the ably-common registry at compile time #2281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ jobs:
- run: npm run lint
- run: npm run format:check
- run: npx tsc --noEmit ably.d.ts modular.d.ts
# src/ is compiled by esbuild, which strips types without checking them, so this is
# the only step that typechecks the library itself.
- run: npx tsc --noEmit -p tsconfig.json
# The error-code union is generated from the ably-common submodule at its pinned
# commit; fail if the committed copy has drifted from the registry.
- run: npm run generate:errorcodes-ts
- run: git diff --exit-code -- src/common/lib/types/errorcodes.ts
# for some reason, this doesn't work in CI using `npx attw --pack .`
- run: npm pack
- run: npx attw ably-$(node -e "console.log(require('./package.json').version)").tgz --summary --exclude-entrypoints 'ably/modular'
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@
"modulereport": "tsc --noEmit --esModuleInterop scripts/moduleReport.ts && esr scripts/moduleReport.ts",
"speccoveragereport": "tsc --noEmit --esModuleInterop --target ES2017 --moduleResolution node scripts/specCoverageReport.ts && esr scripts/specCoverageReport.ts",
"process-private-api-data": "tsc --noEmit --esModuleInterop --strictNullChecks scripts/processPrivateApiData/run.ts && esr scripts/processPrivateApiData/run.ts",
"docs": "typedoc"
"docs": "typedoc",
"generate:errorcodes-ts": "node test/common/ably-common/errors/scripts/generate-ts.js --format=type --out src/common/lib/types/errorcodes.ts"
}
}
17 changes: 14 additions & 3 deletions src/common/lib/client/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Logger from '../util/logger';
import * as Utils from '../util/utils';
import Multicaster, { MulticasterInstance } from '../util/multicaster';
import ErrorInfo, { IPartialErrorInfo } from '../types/errorinfo';
import type { ErrorCode } from '../types/errorcodes';
import { RequestResultError, RequestParams, RequestResult } from '../../types/http';
import * as API from '../../../../ably';
import BaseClient from './baseclient';
Expand All @@ -28,17 +29,27 @@ function isRealtime(client: BaseClient): client is BaseRealtime {
return !!(client as BaseRealtime).connection;
}

/* Fallbacks this SDK supplies when the callback's error carries no code of its own. Annotated
* so they are checked against the registry; the `err.code` below cannot be, because it comes
* from the application's authCallback rather than from this repository. */
const AUTH_CALLBACK_ERROR_CODE: ErrorCode = 40170;
const AUTH_CALLBACK_FORBIDDEN_CODE: ErrorCode = 40300;

/* A client auth callback may give errors in any number of formats; normalise to an ErrorInfo or PartialErrorInfo */
function normaliseAuthcallbackError(err: any) {
if (!Utils.isErrorInfoOrPartialErrorInfo(err)) {
return new ErrorInfo(Utils.inspectError(err), err.code || 40170, err.statusCode || 401);
return new ErrorInfo(
Utils.inspectError(err),
(err.code as ErrorCode) || AUTH_CALLBACK_ERROR_CODE,
err.statusCode || 401,
);
}
Comment on lines 40 to 46

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
fd -a 'auth\.ts$' . | sed 's#^\./##'

echo "== target outline =="
ast-grep outline src/common/lib/client/auth.ts --view expanded || true

echo "== target lines =="
cat -n src/common/lib/client/auth.ts | sed -n '1,120p'

echo "== search auth callback codes and error info helpers =="
rg -n "AUTH_CALLBACK|isErrorInfoOrPartialErrorInfo|40300|40170|statusCode === 403|authCallback" src test -S || true

echo "== deterministic behavior probe from source text =="
python3 - <<'PY'
from pathlib import Path
import re
p=Path('src/common/lib/client/auth.ts')
text=p.read_text()
for name in ['AUTH_CALLBACK_ERROR_CODE','AUTH_CALLBACK_FORBIDDEN_CODE']:
    m=re.search(r'\b'+re.escape(name)+r'\s*=\s*(\d+)',text)
    print(name, '=' , m.group(1) if m else 'NOT_FOUND')
# approximate source branches
print("non_errorinfo_has_status_code_403_check=", bool(re.search(r'!Utils\.isErrorInfoOrPartialErrorInfo\(err\)\s*\{[^}]*statusCode\s*===\s*403', text, re.S)))
print("errorinfo_branch_has_status_code_403_check=", bool(re.search(r'Utils\.isErrorInfoOrPartialErrorInfo\(err\)[^}]*statusCode\s*===\s*403', text, re.S)))
PY

Repository: ably/ably-js

Length of output: 50368

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== normalize helper and call sites =="
sed -n '38,56p' src/common/lib/client/auth.ts | cat -n
rg -n "normaliseAuthcallbackError" src test -S --max-count 50

echo "== targeted test cases for auth callback 403 and code behavior =="
sed -n '239,290p' test/uts/realtime/unit/auth/auth_callback_errors.test.ts | cat -n
sed -n '700,760p' test/realtime/auth.test.js | cat -n

echo "== error code constants =="
sed -n '76,90p' src/common/lib/types/errorcodes.ts | cat -n
sed -n '100,170p' src/common/lib/types/errorcodes.ts | cat -n

echo "== source-derived deterministic behavior models =="
python3 - <<'PY'
from pathlib import Path
for raw, shape in [
    ("raw_403_no_code", {"type_not_errorinfo": True, "statusCode": 403}),
    ("raw_403_with_40170", {"type_not_errorinfo": True, "statusCode": 403, "code": 40170}),
    ("shape_403_no_code", {"type_not_errorinfo": False, "statusCode": 403}),
    ("shape_403_existing_code", {"type_not_errorinfo": False, "statusCode": 403, "code": 80022}),
    ("shape_401_no_code", {"type_not_errorinfo": False, "statusCode": 401}),
]:
    err = dict(shape)
    if not err.get("type_not_errorinfo"):
        err["code"] = 40300 if err["statusCode"] == 403 else 40170
        if err["statusCode"] != 403:
            err["statusCode"] = 401
    else:
        err["code"] = err.get("code") or 40170
    print(raw, "-> code", err["code"], "status", err.get("statusCode"))
PY

Repository: ably/ably-js

Length of output: 8120

Apply the same 403 fallback in the raw authCallback error branch.

A raw authCallback error with statusCode === 403 and no code currently becomes 40170, while an ErrorInfo with the same status gets 40300 as required by the RSA4d tests. Use the 403 fallback here too.

🔧 Possible fix 
   if (!Utils.isErrorInfoOrPartialErrorInfo(err)) {
     return new ErrorInfo(
       Utils.inspectError(err),
-      (err.code as ErrorCode) || AUTH_CALLBACK_ERROR_CODE,
+      (err.code as ErrorCode) || (err.statusCode === 403 ? AUTH_CALLBACK_FORBIDDEN_CODE : AUTH_CALLBACK_ERROR_CODE),
       err.statusCode || 401,
     );
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!Utils.isErrorInfoOrPartialErrorInfo(err)) {
return new ErrorInfo(Utils.inspectError(err), err.code || 40170, err.statusCode || 401);
return new ErrorInfo(
Utils.inspectError(err),
(err.code as ErrorCode) || AUTH_CALLBACK_ERROR_CODE,
err.statusCode || 401,
);
}
if (!Utils.isErrorInfoOrPartialErrorInfo(err)) {
return new ErrorInfo(
Utils.inspectError(err),
(err.code as ErrorCode) || (err.statusCode === 403 ? AUTH_CALLBACK_FORBIDDEN_CODE : AUTH_CALLBACK_ERROR_CODE),
err.statusCode || 401,
);
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/common/lib/client/auth.ts` around lines 40 - 46, Update the raw error
branch in authCallback handling to use the 403-specific fallback error code when
err.statusCode is 403 and no err.code is present, while retaining
AUTH_CALLBACK_ERROR_CODE for other statuses and existing codes.

/* network errors will not have an inherent error code */
if (!err.code) {
if (err.statusCode === 403) {
err.code = 40300;
err.code = AUTH_CALLBACK_FORBIDDEN_CODE;
} else {
err.code = 40170;
err.code = AUTH_CALLBACK_ERROR_CODE;
/* normalise statusCode to 401 per RSA4e */
err.statusCode = 401;
}
Expand Down
4 changes: 2 additions & 2 deletions src/common/lib/client/realtimechannel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ class RealtimeChannel extends EventEmitter {

case actions.DETACHED: {
const detachErr = message.error
? ErrorInfo.fromValues(message.error)
? ErrorInfo.fromWireValues(message.error)
: new ErrorInfo('Channel detached', 90001, 404);
if (this.state === 'detaching') {
this.notifyState('detached', detachErr);
Expand Down Expand Up @@ -858,7 +858,7 @@ class RealtimeChannel extends EventEmitter {
/* attach/detach operation attempted on superseded transport handle */
this.checkPendingState();
} else {
this.notifyState('failed', ErrorInfo.fromValues(err));
this.notifyState('failed', ErrorInfo.fromWireValues(err));
}
break;
}
Expand Down
11 changes: 8 additions & 3 deletions src/common/lib/transport/connectionerrors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import ErrorInfo from '../types/errorinfo';
import type { ErrorCode } from '../types/errorcodes';

// `satisfies` checks every code against the registry while keeping the literal types, so
// that the members stay usable as ErrorCode at the call sites below.
const ConnectionErrorCodes = {
DISCONNECTED: 80003,
SUSPENDED: 80002,
Expand All @@ -8,7 +11,7 @@ const ConnectionErrorCodes = {
CLOSED: 80017,
UNKNOWN_CONNECTION_ERR: 50002,
UNKNOWN_CHANNEL_ERR: 50001,
};
} as const satisfies Record<string, ErrorCode>;

const ConnectionErrors = {
disconnected: () =>
Expand Down Expand Up @@ -50,7 +53,7 @@ const ConnectionErrors = {
unknownChannelErr: () =>
ErrorInfo.fromValues({
statusCode: 500,
code: ConnectionErrorCodes.UNKNOWN_CONNECTION_ERR,
code: ConnectionErrorCodes.UNKNOWN_CHANNEL_ERR,
message: 'Internal channel error',
}),
};
Expand All @@ -59,7 +62,9 @@ export function isRetriable(err: ErrorInfo) {
if (!err.statusCode || !err.code || err.statusCode >= 500) {
return true;
}
return Object.values(ConnectionErrorCodes).includes(err.code);
// Widened because `as const` gives the values their own narrow literal union, which
// `includes` will not accept an arbitrary ErrorCode against.
return (Object.values(ConnectionErrorCodes) as ErrorCode[]).includes(err.code);
}

export default ConnectionErrors;
2 changes: 1 addition & 1 deletion src/common/lib/types/devicedetails.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class DeviceDetails {
}

static fromValues(values: Record<string, unknown>): DeviceDetails {
values.error = values.error && ErrorInfo.fromValues(values.error as IConvertibleToErrorInfo);
values.error = values.error && ErrorInfo.fromWireValues(values.error as IConvertibleToErrorInfo);
return Object.assign(new DeviceDetails(), values);
}

Expand Down
266 changes: 266 additions & 0 deletions src/common/lib/types/errorcodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,266 @@
// GENERATED FROM ably-common/errors/codes — DO NOT EDIT.
// Regenerate with: npm run generate:errorcodes-ts

/** A registered Ably error code. */
export type ErrorCode =
| 10000
| 20000
| 40000
| 40001
| 40002
| 40003
| 40004
| 40005
| 40006
| 40007
| 40008
| 40009
| 40010
| 40011
| 40012
| 40013
| 40014
| 40015
| 40016
| 40017
| 40018
| 40019
| 40020
| 40021
| 40022
| 40023
| 40024
| 40025
| 40030
| 40031
| 40032
| 40033
| 40034
| 40035
| 40099
| 40100
| 40101
| 40102
| 40103
| 40104
| 40105
| 40106
| 40110
| 40111
| 40112
| 40113
| 40114
| 40115
| 40120
| 40121
| 40125
| 40126
| 40127
| 40128
| 40130
| 40131
| 40132
| 40133
| 40140
| 40141
| 40142
| 40143
| 40144
| 40145
| 40150
| 40151
| 40160
| 40161
| 40162
| 40163
| 40164
| 40165
| 40166
| 40167
| 40170
| 40171
| 40172
| 40180
| 40181
| 40182
| 40300
| 40310
| 40311
| 40320
| 40330
| 40331
| 40332
| 40400
| 40500
| 40900
| 41001
| 42200
| 42210
| 42211
| 42212
| 42213
| 42910
| 42911
| 42912
| 42913
| 42914
| 42915
| 42916
| 42917
| 42918
| 42920
| 42921
| 42922
| 42923
| 42924
| 42925
| 42926
| 50000
| 50001
| 50002
| 50003
| 50004
| 50005
| 50006
| 50010
| 50205
| 50210
| 50305
| 50306
| 50310
| 50320
| 50330
| 50405
| 50410
| 61002
| 70000
| 70001
| 70002
| 70003
| 70004
| 70005
| 70006
| 71000
| 71001
| 71100
| 71101
| 71102
| 71200
| 71201
| 71202
| 71203
| 71204
| 71300
| 71301
| 71302
| 71303
| 72000
| 72001
| 72002
| 72003
| 72004
| 72005
| 72006
| 72007
| 72008
| 80000
| 80001
| 80002
| 80003
| 80004
| 80005
| 80006
| 80007
| 80008
| 80009
| 80010
| 80011
| 80012
| 80013
| 80014
| 80015
| 80016
| 80017
| 80018
| 80019
| 80020
| 80021
| 80022
| 80023
| 80024
| 80030
| 90000
| 90001
| 90002
| 90003
| 90004
| 90005
| 90006
| 90007
| 90008
| 90009
| 90010
| 90021
| 91000
| 91001
| 91002
| 91003
| 91004
| 91005
| 91006
| 91007
| 91008
| 91100
| 92000
| 92001
| 92002
| 92003
| 92004
| 92005
| 92006
| 92007
| 92008
| 93001
| 93002
| 101000
| 101001
| 101002
| 101003
| 101004
| 102000
| 102001
| 102002
| 102003
| 102004
| 102005
| 102050
| 102051
| 102052
| 102053
| 102054
| 102100
| 102101
| 102102
| 102103
| 102104
| 102105
| 102106
| 102107
| 102108
| 102109
| 102110
| 102111
| 102112
| 102113
| 102200
| 102201
| 102202
| 103000
| 103001
| 103002
| 103003
| 103004
| 103005
| 103006
| 103007
| 103008;
Loading
Loading