Skip to content
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.

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

undefined err in node_transport.ts #441

Closed
niranjannitesh opened this issue Aug 18, 2021 · 40 comments
Closed

undefined err in node_transport.ts #441

niranjannitesh opened this issue Aug 18, 2021 · 40 comments
Assignees

Comments

@niranjannitesh
Copy link

TypeError: Cannot destructure property 'code' of 'err' as it is undefined.
    at NodeTransport.<anonymous> (/usr/app/packages/shared/node_modules/nats/src/node_transport.ts:80:15)
    at Generator.throw (<anonymous>)
    at rejected (/usr/app/packages/shared/node_modules/nats/lib/src/node_transport.js:6:65)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
@aricart aricart self-assigned this Aug 18, 2021
@aricart
Copy link
Member

aricart commented Aug 18, 2021

@niteshkumarniranjan how is that even possible?

      return Promise.resolve();
    } catch (err) {
      const { code } = err;
      const perr = code === "ECONNREFUSED"
        ? NatsError.errorForCode(ErrorCode.ConnectionRefused, err)
        : err;
      if (this.socket) {
        this.socket.destroy();
      }
      throw perr;
    }

The code is clearly catching an exception so the only thing that could happen is for code to be undefined.
Any idea of what the error was or how you got to it?

@aricart aricart closed this as completed Aug 18, 2021
@aricart aricart reopened this Aug 18, 2021
@aricart
Copy link
Member

aricart commented Aug 18, 2021

Are you building your own version of the library? The build steps are very peculiar, and the error you have is from typescript.

@niranjannitesh
Copy link
Author

niranjannitesh commented Aug 19, 2021

Maybe, the error is from https://github.com/nats-io/nats.deno

It's mentioned in the doc that the nats.js library shares client functionality with NATS.deno. I didn't know where to open the issue.

And the error happens in k8s environment when the NATS pod isn't ready and my client is trying to connect.

@niranjannitesh
Copy link
Author

niranjannitesh commented Aug 19, 2021

Just to add a bit of context, I am running my code using ts-node-dev
I have written a wrapper

import * as nats from "nats"
import rTracer from "cls-rtracer"

const sc = nats.JSONCodec()

function encode<T>(obj: { data: T; traceId: unknown }) {
  return sc.encode(obj)
}

function decode<T>(str: Uint8Array) {
  return sc.decode(str) as { data: T; traceId: unknown }
}

export interface IBus {
  subscribe<T>(
    name: string,
    callback: (arg: {
      obj: { data: T; traceId: unknown }
      m: nats.JsMsg
    }) => void
  ): Promise<void>
  publish<T>(name: string, data: T): Promise<void>
}

export default function createNATSTransport({
  servers,
  durableName,
  logger,
}: {
  servers: string | string[]
  durableName: string
  logger: any
}) {
  let isConnected = false
  let nc: nats.NatsConnection,
    jsm: nats.JetStreamManager,
    js: nats.JetStreamClient

  async function connect() {
    nc = await nats.connect({ servers, name: durableName, timeout: 30 * 1000 })
    jsm = await nc.jetstreamManager()
    js = await nc.jetstream()
    logger.info({
      msg: `connected to nats`,
      url: servers,
    })
    isConnected = true
  }

  async function drain() {
    await nc.drain()
  }

  const bus: IBus = {
    async subscribe<T>(
      name: string,
      callback: (arg: {
        obj: { data: T; traceId: unknown }
        m: nats.JsMsg
      }) => void
    ) {
      if (!isConnected) {
        throw new Error("NATS is not connected")
      }
      const streams = await jsm.streams.list().next()
      const [stream, ...extra] = name.split(".")
      const streamExists =
        streams.findIndex((x) => x.config.name === stream) > -1
      if (!streamExists) {
        await jsm.streams.add({ name: stream, subjects: [`${stream}.*`] })
      }
      const inbox = nats.createInbox()
      const opts = nats.consumerOpts({
        ack_policy: nats.AckPolicy.Explicit,
        deliver_policy: nats.DeliverPolicy.Last,
        deliver_subject: inbox,
        flow_control: true,
        durable_name: durableName,
      })
      const sub = await js.subscribe(name, { ...opts, queue: durableName })
      ;(async () => {
        for await (const m of sub) {
          const obj = decode<T>(m.data)
          logger.info({
            msg: "bus event recieved",
            event: {
              name,
              ...obj,
            },
          })
          callback({
            obj,
            m: m,
          })
        }
      })()
    },
    async publish<T>(name: string, data: T) {
      if (!isConnected) {
        throw new Error("NATS is not connected")
      }

      const streams = await jsm.streams.list().next()
      const [stream, ...extra] = name.split(".")
      const streamExists =
        streams.findIndex((x) => x.config.name === stream) > -1
      if (!streamExists) {
        await jsm.streams.add({ name: stream, subjects: [`${stream}.*`] })
      }

      const wrappedData = {
        data,
        traceId: rTracer.id(),
      }
      js.publish(name, encode<T>(wrappedData))
      logger.info({
        msg: `bus event published`,
        event: {
          name,
        },
      })
    },
  }

  const transport = {
    reply<RequestData, ResponseData>(
      subject: string,
      callback: (obj: {
        data: RequestData
        traceId: unknown
      }) => ResponseData | Promise<ResponseData>
    ) {
      if (!isConnected) {
        throw new Error("NATS is not connected")
      }
      const sub = nc.subscribe("x" + subject, { queue: durableName })
      ;(async () => {
        for await (const m of sub) {
          const obj = decode<RequestData>(m.data)
          logger.info({
            msg: "nats transport request recieved",
            event: {
              name: subject,
              ...obj,
            },
          })
          const result = await callback(obj)
          const encodedData = encode<ResponseData>({
            data: result,
            traceId: obj.traceId,
          })
          if (m.respond(encodedData)) {
            logger.info({
              msg: "nats transport reply",
              traceId: obj.traceId,
              event: {
                name: subject,
              },
            })
          }
        }
      })()
    },
    async request<RequestData, ResponseData>(
      subject: string,
      data: RequestData
    ) {
      if (!isConnected) {
        throw new Error("NATS is not connected")
      }
      const traceId = rTracer.id()
      const response = await nc.request(
        "x" + subject,
        encode<RequestData>({ data, traceId })
      )

      return decode<ResponseData>(response.data)
    },
  }

  return {
    connect,
    bus,
    transport,
    drain,
  }
}

@aricart
Copy link
Member

aricart commented Aug 20, 2021

@niteshkumarniranjan please don't run with ts-node, the code has to be processed by the build in order to generate javascript - there's some source manipulation etc that happens during the build process which ts-node is not able to do. Please use the released npm bundle.

@aricart aricart closed this as completed Aug 20, 2021
@niranjannitesh
Copy link
Author

@aricart I am using the published npm version only.

@aricart aricart reopened this Aug 20, 2021
@aricart
Copy link
Member

aricart commented Aug 20, 2021

I am at a loss on how a catch(err) block can be involved, but the next line says that err is undefined.... On the destructure, the only thing that can happen is that code could be undefined, but that is OK.

@niranjannitesh
Copy link
Author

@aricart I'll try to do some debugging and will get back if I find something.

@niranjannitesh
Copy link
Author

on all socket close events, it is assumed that the error variable is defined

d.reject(peekError);

d.reject(tlsError);

d.reject(dialError);

Might be a dumb question but is it possible that the socket connection is being closed without any errors?

@aricart
Copy link
Member

aricart commented Aug 20, 2021

even if that was true, the issue is that the catch block is invoked, how is it possible for a throw undefined

@aricart
Copy link
Member

aricart commented Aug 20, 2021

Can you modify the code and simply put a console.log on the err console.log(err)

@niranjannitesh
Copy link
Author

I mean you can reject a promise without any error

const a = (x) => {
  return new Promise((resolve, reject) => {
    if (x) resolve(a)
    else reject()
  })
}

async function main() {
  try {
    await a()
  } catch (err) {
    const {
      code
    } = err
    console.log(code)
  }
}

main()

@niranjannitesh
Copy link
Author

I am not sure if I can modify the code for a module inside node_modules inside a docker image?

@aricart
Copy link
Member

aricart commented Aug 20, 2021

Yes the reject simply means that the promise failed, but no error. But the code you are pointing out, is a catch...

@aricart
Copy link
Member

aricart commented Aug 20, 2021

you would have to modify the npm package before you copy it to the docker image

@niranjannitesh
Copy link
Author

yes!

const { code } = err;

here the only awaits that are being done are await this.dial(hp); await this.peekInfo(); and this.startTLS();

And you are using deferred reject inside dial & peekInfo and startTLS
which is a promise.reject, isn't it?

@niranjannitesh
Copy link
Author

you would have to modify the npm package before you copy it to the docker image

Let me try.

@aricart
Copy link
Member

aricart commented Aug 20, 2021

I am going to post a version for you to use

@niranjannitesh
Copy link
Author

Yup! it is undefined

@aricart
Copy link
Member

aricart commented Aug 20, 2021

something changed in node
what version are you using?

@niranjannitesh
Copy link
Author

14.17.4

@aricart
Copy link
Member

aricart commented Aug 20, 2021

are you in slack?

@niranjannitesh
Copy link
Author

Nope!

@aricart
Copy link
Member

aricart commented Aug 20, 2021

you are saying when this happens the nats-server is not ready or kubernetes node is not yet running right?

@niranjannitesh
Copy link
Author

I am also using istio sidecar with all my pods.
so when everything boots up, my node.js client tries to connect with NATS while either NATS or istio-init is booting up
and I see this error then only.

@aricart
Copy link
Member

aricart commented Aug 20, 2021

are you using any library that is looking or inspecting any network traffic?

@niranjannitesh
Copy link
Author

inside node.js? no

@aricart
Copy link
Member

aricart commented Aug 20, 2021

one more, can you try with the current version of node - I will do a dev release for an npm bundle that treats this as a connection refused. But would like to know if current node version is affected.

@niranjannitesh
Copy link
Author

A/C to the node.js doc for net
https://nodejs.org/api/net.html#net_event_close_1

when the close event is emitted it also provides if was because of an error.
My guess is that the socket is being closed without any errors because of istio-init is booting up

@aricart
Copy link
Member

aricart commented Aug 20, 2021

weird how we have not seen this earlier.

@niranjannitesh
Copy link
Author

niranjannitesh commented Aug 20, 2021

I checked with the latest node.js, still has the same error.

@niranjannitesh
Copy link
Author

I don't know how NATS Server behaves,
but when using istio as a sidecar, it basically hijacks the whole connection, which means istio can close the socket connection to the node.js client

@aricart
Copy link
Member

aricart commented Aug 20, 2021

ok I just published a bundle - can younpm install nats@dev - that will give you 2.1.1-1.

@niranjannitesh
Copy link
Author

okay! testing

@niranjannitesh
Copy link
Author

NatsError: CONNECTION_REFUSED
at Function.errorForCode (/usr/app/packages/shared/node_modules/nats/nats-base-client/error.ts:115:12)
at NodeTransport.<anonymous> (/usr/app/packages/shared/node_modules/nats/src/node_transport.ts:83:25)
at Generator.throw (<anonymous>)
at rejected (/usr/app/packages/shared/node_modules/nats/lib/src/node_transport.js:6:65)
at processTicksAndRejections (internal/process/task_queues.js:95:5) {
code: 'CONNECTION_REFUSED',
chainedError: Error: node provided an undefined error!
at NodeTransport.<anonymous> (/usr/app/packages/shared/node_modules/nats/src/node_transport.ts:85:11)
at Generator.throw (<anonymous>)
at rejected (/usr/app/packages/shared/node_modules/nats/lib/src/node_transport.js:6:65)
at processTicksAndRejections (internal/process/task_queues.js:95:5)
}

@aricart
Copy link
Member

aricart commented Aug 20, 2021

ok, so now, in your code provide the waitOnFirstConnect=true option, and if you have the reconnectTimeWait: 1000 to set to something like 1 second, the client should continue to attempt to reconnect.

aricart added a commit that referenced this issue Aug 20, 2021
…close, triggering a catch(err) where the error is undefined - this fix simply aliases that as a connection refused error.
@niranjannitesh
Copy link
Author

Since I am running on k8s, my pod just restarts in a couple of seconds and it connects successfully since NATS is up and running by then.

@aricart
Copy link
Member

aricart commented Aug 20, 2021

there will be a release of nats clients for javascript next week - waiting on a server release, this will be included it in it.

@aricart aricart mentioned this issue Aug 20, 2021
@aricart
Copy link
Member

aricart commented Aug 20, 2021

@niteshkumarniranjan thank you for helping out on this!

@aricart aricart closed this as completed Aug 20, 2021
@niranjannitesh
Copy link
Author

happy to help!

derekcollison added a commit that referenced this issue Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants