Skip to content

Commit

Permalink
[fix] normalized error code handling:
Browse files Browse the repository at this point in the history
  - codes are string representation of the number
  - description is lowercased string
  - constants simply reference the string code
  • Loading branch information
aricart committed Mar 13, 2021
1 parent f792868 commit c83a9dc
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 58 deletions.
10 changes: 5 additions & 5 deletions nats-base-client/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export enum ErrorCode {
InvalidOption = "INVALID_OPTION",
InvalidPayload = "INVALID_PAYLOAD",
MaxPayloadExceeded = "MAX_PAYLOAD_EXCEEDED",
NoResponders = "NO_RESPONDERS",
NoResponders = "503",
NotFunction = "NOT_FUNC",
RequestError = "REQUEST_ERROR",
ServerOptionNotAvailable = "SERVER_OPT_NA",
Expand All @@ -43,11 +43,11 @@ export enum ErrorCode {
WssRequired = "WSS_REQUIRED",

// jetstream
JetStreamNotEnabled = "JETSTREAM_NOT_ENABLED",
JetStreamInvalidAck = "JESTREAM_INVALID_ACK",
JetStream404NoMessages = "404_NO_MESSAGES",
JetStream408RequestTimeout = "408_REQUEST_TIMEOUT",
JetStream409MaxAckPendingExceeded = "409_MAX_ACK_PENDING_EXCEEDED",
JetStream404NoMessages = "404",
JetStream408RequestTimeout = "408",
JetStream409MaxAckPendingExceeded = "409",
JetStreamNotEnabled = "503",

// emitted by the server
AuthorizationViolation = "AUTHORIZATION_VIOLATION",
Expand Down
12 changes: 4 additions & 8 deletions nats-base-client/jsbaseclient_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import {
StreamNames,
} from "./types.ts";
import { Codec, JSONCodec } from "./codec.ts";
import { ErrorCode, NatsError } from "./error.ts";
import { extend } from "./util.ts";
import { NatsConnectionImpl } from "./nats.ts";
import { checkJsErrorCode } from "./jsutil.ts";

const defaultPrefix = "$JS.API";
const defaultTimeout = 5000;
Expand Down Expand Up @@ -98,15 +98,11 @@ export class BaseApiClient {
parseJsResponse(m: Msg): unknown {
const v = this.jc.decode(m.data);
const r = v as ApiResponse;

if (r.error) {
if (r.error.code === 503) {
throw NatsError.errorForCode(
ErrorCode.JetStreamNotEnabled,
new Error(r.error.description),
);
const err = checkJsErrorCode(r.error.code, r.error.description);
if (err !== null) {
throw err;
}
throw new NatsError(r.error.description, `${r.error.code}`);
}
return v;
}
Expand Down
40 changes: 8 additions & 32 deletions nats-base-client/jsclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ import {
RequestOptions,
} from "./types.ts";
import { BaseApiClient } from "./jsbaseclient_api.ts";
import { nanos, validateDurableName, validateStreamName } from "./jsutil.ts";
import {
checkJsError,
isFlowControlMsg,
nanos,
validateDurableName,
validateStreamName,
} from "./jsutil.ts";
import { ConsumerAPIImpl } from "./jsconsumer_api.ts";
import { toJsMsg } from "./jsmsg.ts";
import {
Expand All @@ -53,8 +59,8 @@ import { QueuedIterator, QueuedIteratorImpl } from "./queued_iterator.ts";
import { Timeout, timeout } from "./util.ts";
import { createInbox } from "./protocol.ts";
import { headers } from "./headers.ts";
import { consumerOpts, isConsumerOptsBuilder } from "./consumeropts.ts";
import type { ConsumerOptsBuilder } from "./consumeropts.ts";
import { consumerOpts, isConsumerOptsBuilder } from "./consumeropts.ts";

export interface JetStreamSubscriptionInfoable {
info: JetStreamSubscriptionInfo | null;
Expand Down Expand Up @@ -521,33 +527,3 @@ function autoAckJsMsg(data: JsMsg | null) {
data.ack();
}
}

function isFlowControlMsg(msg: Msg): boolean {
const h = msg.headers;
if (!h) {
return false;
}
return h.code >= 100 && h.code < 200;
}

function checkJsError(msg: Msg): NatsError | null {
const h = msg.headers;
if (!h) {
return null;
}
if (h.code < 300) {
return null;
}
switch (h.code) {
case 404:
return NatsError.errorForCode(ErrorCode.JetStream404NoMessages);
case 408:
return NatsError.errorForCode(ErrorCode.JetStream408RequestTimeout);
case 409:
return NatsError.errorForCode(
ErrorCode.JetStream409MaxAckPendingExceeded,
);
default:
return NatsError.errorForCode(ErrorCode.Unknown, new Error(h.status));
}
}
40 changes: 40 additions & 0 deletions nats-base-client/jsutil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import {
AckPolicy,
ConsumerConfig,
DeliverPolicy,
Msg,
Nanos,
ReplayPolicy,
} from "./types.ts";
import { ErrorCode, NatsError } from "./error.ts";

export function validateDurableName(name?: string) {
return validateName("durable", name);
Expand Down Expand Up @@ -62,3 +64,41 @@ export function nanos(millis: number): Nanos {
export function millis(ns: Nanos) {
return ns / 1000000;
}

export function isFlowControlMsg(msg: Msg): boolean {
const h = msg.headers;
if (!h) {
return false;
}
return h.code >= 100 && h.code < 200;
}

export function checkJsError(msg: Msg): NatsError | null {
const h = msg.headers;
if (!h) {
return null;
}
return checkJsErrorCode(h.code, h.status);
}

export function checkJsErrorCode(
code: number,
description = "",
): NatsError | null {
if (code < 300) {
return null;
}
description = description.toLowerCase();
switch (code) {
case 503:
return NatsError.errorForCode(
ErrorCode.JetStreamNotEnabled,
new Error(description),
);
default:
if (description === "") {
description = ErrorCode.Unknown;
}
return new NatsError(description, `${code}`);
}
}
14 changes: 7 additions & 7 deletions nats-base-client/msg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@ import type { MsgArg } from "./parser.ts";
import { TD } from "./encoders.ts";
import { ErrorCode, NatsError } from "./error.ts";

const noResponders = "503";

export function isRequestError(msg: Msg): (NatsError | null) {
if (msg && msg.headers) {
const headers = msg.headers as MsgHdrsImpl;
if (headers.hasError) {
if (headers.status === noResponders) {
if (headers.status === "503") {
return NatsError.errorForCode(ErrorCode.NoResponders);
} else {
return new NatsError(
headers.status,
ErrorCode.RequestError,
);
let desc = headers.get("description");
if (desc === "") {
desc = ErrorCode.RequestError;
}
desc = desc.toLowerCase();
return new NatsError(desc, headers.status);
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions tests/jetstream_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ Deno.test("jetstream - publish require last message id", async () => {
await js.publish(subj, Empty, { msgID: "b", expect: { lastMsgID: "b" } });
},
Error,
"wrong last msg ID: a",
"wrong last msg id: a",
);

pa = await js.publish(subj, Empty, {
Expand Down Expand Up @@ -335,12 +335,12 @@ Deno.test("jetstream - pull no messages", async () => {
ack_policy: AckPolicy.Explicit,
});
const js = nc.jetstream();
assertThrowsAsync(
await assertThrowsAsync(
async () => {
await js.pull(stream, "me");
},
Error,
"404 No Messages",
"no messages",
);

await cleanup(ns, nc);
Expand Down Expand Up @@ -384,7 +384,7 @@ Deno.test("jetstream - pull batch no messages", async () => {
await js.pull(stream, "me");
},
Error,
"404 No Messages",
"no messages",
);
await cleanup(ns, nc);
});
Expand Down Expand Up @@ -1284,7 +1284,7 @@ Deno.test("jetstream - cross account pull", async () => {
await js.pull(stream, "me");
},
Error,
"No Messages",
"no messages",
);

await cleanup(ns, admin, nc);
Expand Down
2 changes: 1 addition & 1 deletion tests/jsm_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ Deno.test("jsm - get message", async () => {
await jsm.streams.getMessage(stream, 3);
},
Error,
"stream store EOF",
"stream store eof",
);

await cleanup(ns, nc);
Expand Down

0 comments on commit c83a9dc

Please sign in to comment.