node icon indicating copy to clipboard operation
node copied to clipboard

SyntaxError: Unexpected token ; in JSON at position 0

Open sayhicoelho opened this issue 1 year ago • 5 comments

My server just crashed after few months using this library.

I'm using try/catch, but I think the error was caused by an event listener block and my code couldn't handle it.

Log:

undefined:1
;{
^

SyntaxError: Unexpected token ; in JSON at position 0
    at JSON.parse (<anonymous>)
    at IncomingMessage.<anonymous> (/**/node_modules/node-ipinfo/dist/src/ipinfoWrapper.js:109:67)
    at IncomingMessage.emit (node:events:517:28)
    at emitCloseNT (node:internal/streams/destroy:132:10)
    at process.processTicksAndRejections (node:internal/process/task_queues:81:21)

Node.js v18.20.4

error Command failed with exit code 1.

To avoid these crashes, it would be good to have the try/catch block in JSON.parse on these lines:

113: https://github.com/ipinfo/node/blob/v3.5.1/src/ipinfoWrapper.ts#L113 210: https://github.com/ipinfo/node/blob/v3.5.1/src/ipinfoWrapper.ts#L210 293: https://github.com/ipinfo/node/blob/v3.5.1/src/ipinfoWrapper.ts#L293 451: https://github.com/ipinfo/node/blob/v3.5.1/src/ipinfoWrapper.ts#L451

Version:

node-ipinfo: "^3.5.1"

sayhicoelho avatar Jul 12 '24 18:07 sayhicoelho

I made a code example to reproduce this issue:

index.js

const locationService = require('./services/location-service.js')

async function main() {
  let location

  try {
    location = await locationService.getFakeLocation()
  } catch (err) {
    location = null
  }

  console.log({
    location
  })
}

main()

services/location-service.js

const { EventEmitter } = require('events')

exports.getFakeLocation = () => {
  try {
    return new Promise((resolve, reject) => {
      const req = fakeRequest(res => {
        res.on('data', json => {
          const data = JSON.parse(json)
          resolve(data)
        })
      })

      req.on('error', err => {
        reject(err)
      })
    })
  } catch (err) {
    console.error(err)
  }
}

function fakeRequest(callback) {
  const req = new EventEmitter()
  const res = new EventEmitter()

  callback(res)

  setTimeout(() => {
    res.emit('data', ';{"country": "United States"}')
  }, 2000)

  // setTimeout(() => {
  //   req.emit('error', new Error('Error example'))
  // }, 2000)

  return req
}

Output:

undefined:1
;{"country": "United States"}
^

SyntaxError: Unexpected token ; in JSON at position 0
    at JSON.parse (<anonymous>)
    at EventEmitter.<anonymous> (**/services/location-service.js:8:29)
    at EventEmitter.emit (node:events:514:28)
    at Timeout._onTimeout (**/services/location-service.js:29:9)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7)

Node.js v18.17.1

As you can see, my catch block is never reached. The application just crashes.

sayhicoelho avatar Jul 12 '24 20:07 sayhicoelho

Hey @sayhicoelho, we are looking into this matter. In our first attempts, we were not able to reproduce the bug. Would you please reach out to us via support? We need to evaluate the IPs you were testing with and might request you share some additional information. Reaching out through support would help us get that information effectively.

🔗 https://ipinfo.io/contact

abdullahdevrel avatar Jul 16 '24 00:07 abdullahdevrel

@abdullahdevrel I'm experiencing the same. Here's the only code in my codebase that touches ipinfo.

import * as envalid from 'envalid';
import logger from 'libs/logger';
import IPinfoWrapper, { ApiLimitError, IPinfo } from 'node-ipinfo';

const { IPINFO_TOKEN } = envalid.cleanEnv(process.env, {
  IPINFO_TOKEN: envalid.str(),
});

const ipinfoWrapper = new IPinfoWrapper(IPINFO_TOKEN);

export const getGeolocationForIP = async (ip: string | undefined | null): Promise<IPinfo | undefined> => {
  if (!ip) return undefined;

  try {
    return ipinfoWrapper.lookupIp(ip);
  } catch (error) {
    if (error instanceof ApiLimitError) {
      logger.error('ipinfo rate limit exceeded', { ip, error });
    } else {
      logger.warn('ipinfo errored looking up ip geolocation', { ip, error });
    }
    return undefined;
  }
};
(base) ➜  api git:(master) yarn why node-ipinfo
yarn why v1.22.22
[1/4] 🤔  Why do we have the module "node-ipinfo"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Has been hoisted to "node-ipinfo"
info This module exists because it's specified in "dependencies".
info Disk size without dependencies: "376KB"
info Disk size with unique dependencies: "376KB"
info Disk size with transitive dependencies: "376KB"
info Number of shared dependencies: 3
✨  Done in 0.14s.

Screenshot 2024-08-20 at 13 57 01

14 production crashes here from just today

I'm on an older version of this package 3.1.0, but do you think upgrading would even fix it or is this a deeper issue?

tsheaff avatar Aug 21 '24 07:08 tsheaff

@tsheaff I let the team know and they are actively working on a solution at this moment. When the issue was opened we pushed some fixes and were monitoring the situation.

From the initial discussion it looks to be a Google Cloud platform related issue of some sort and the probable inconsistent nature of error statements in usual circumstances. I will keep you posted.

abdullahdevrel avatar Aug 22 '24 08:08 abdullahdevrel

Forgot to post an update. This issue should be resolved by updating to the latest release. Can you please confirm if the update solves your issue?

abdullahdevrel avatar Aug 26 '24 10:08 abdullahdevrel

@sayhicoelho I just pushed a fix to https://www.npmjs.com/package/node-ipinfo/v/3.5.5

Thank you for your patience!

max-ipinfo avatar Nov 08 '24 04:11 max-ipinfo