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

JSON parsing issue #7

Closed
yocontra opened this issue Feb 2, 2018 · 18 comments
Closed

JSON parsing issue #7

yocontra opened this issue Feb 2, 2018 · 18 comments
Labels
help wanted Extra attention is needed

Comments

@yocontra
Copy link

yocontra commented Feb 2, 2018

Just tried to run this for the first time and got this on pressing ctrl-c, not sure how to debug it further for ya:

analysing data
events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: Unexpected COMMA(",") in state COLON
    at Parser.proto.parseError (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/jsonparse/jsonparse.js:321:16)
    at Parser.proto.onToken (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/jsonparse/jsonparse.js:395:24)
    at Parser.parser.onToken (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/JSONStream/index.js:126:12)
    at Parser.proto.write (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/jsonparse/jsonparse.js:139:34)
    at Stream.<anonymous> (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/JSONStream/index.js:21:12)
    at Stream.stream.write (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/through/index.js:26:11)
    at TraceEventDecoder._transform (/Users/contra/.nvm/versions/node/v8.9.4/lib/node_modules/clinic/node_modules/@nearform/clinic-doctor/format/trace-event-decoder.js:114:19)
    at TraceEventDecoder.Transform._read (_stream_transform.js:186:10)
    at TraceEventDecoder.Transform._write (_stream_transform.js:174:12)
    at doWrite (_stream_writable.js:387:12)

then looking at the folder, it spit out 30+ folders:

screen shot 2018-02-01 at 11 28 47 pm

might have something to do with happypack running a bunch of subprocesses

@AndreasMadsen
Copy link
Contributor

might have something to do with happypack running a bunch of subprocesses

Yep, that sounds reasonable. What do you think the expected behavior should be?

@AndreasMadsen
Copy link
Contributor

As for the Error: Unexpected COMMA(",") in state COLON issue. There is unfortunately some bugs in node.js v8.9.4, we have fixed then but node.js v8.10.0 hasn't been released yet.

You can try with node.js v9.4.0 that should work better.

@yocontra
Copy link
Author

yocontra commented Feb 2, 2018

From what I'm seeing is that when the traces get very large this error pops up - if it's a node 8 thing I'll close it and try with node 9.

@yocontra yocontra closed this as completed Feb 2, 2018
@AndreasMadsen AndreasMadsen added the help wanted Extra attention is needed label Feb 3, 2018
@gboysko
Copy link

gboysko commented Feb 9, 2018

I'm using the latest clinic doctor with Node.js 9.5.0 and am getting a slightly different JSON error during analysis:

^Canalysing data
events.js:137
      throw er; // Unhandled 'error' event
      ^

Error: Invalid JSON (Unexpected "\"" at position 20385 in state STOP)
    at Parser.proto.charError (/Users/boysko/.nvm/versions/node/v9.5.0/lib/node_modules/clinic/node_modules/jsonparse/jsonparse.js:90:16)
    at Parser.proto.write (/Users/boysko/.nvm/versions/node/v9.5.0/lib/node_modules/clinic/node_modules/jsonparse/jsonparse.js:284:26)
    at Stream.<anonymous> (/Users/boysko/.nvm/versions/node/v9.5.0/lib/node_modules/clinic/node_modules/JSONStream/index.js:21:12)
    at Stream.stream.write (/Users/boysko/.nvm/versions/node/v9.5.0/lib/node_modules/clinic/node_modules/through/index.js:26:11)
    at TraceEventDecoder._transform (/Users/boysko/.nvm/versions/node/v9.5.0/lib/node_modules/clinic/node_modules/@nearform/clinic-doctor/format/trace-event-decoder.js:114:19)
    at TraceEventDecoder.Transform._read (_stream_transform.js:185:10)
    at TraceEventDecoder.Transform._write (_stream_transform.js:173:12)
    at doWrite (_stream_writable.js:407:12)
    at writeOrBuffer (_stream_writable.js:393:5)
    at TraceEventDecoder.Writable.write (_stream_writable.js:290:11)

Would it help if I sent you the generated files?

@davidmarkclements
Copy link
Contributor

@gboysko are you using the cluster module?

@gboysko
Copy link

gboysko commented Feb 9, 2018

I'm using the sticky-cluster module in my application (which uses cluster internally). So, I guess, yes. Is that a limitation?

@AndreasMadsen
Copy link
Contributor

@gboysko Each process will write to the same file which is causing issues. We are working on fixing it in nodejs/node#18480 – However, even when "fixed" it won't be obvious what data from what process you want to be analyzed. Thus the best thing will properly always be to benchmark a single standalone-worker.

@AndreasMadsen
Copy link
Contributor

I created an issue to track the cluster issue: https://github.com/nearform/node-clinic-doctor/issues/102

@gboysko
Copy link

gboysko commented Feb 9, 2018

Thanks @AndreasMadsen. I can startup my application with a single worker process, but don't know if I could instruct Clinic to only monitor it. Is that possible? I could add calls directly into the child process workflow if the command line tool doesn't have the ability.

@AndreasMadsen
Copy link
Contributor

For the moment you need to run the worker without a master process. If the master is decoupled from the worker by using exec in cluster.setupMaster then that should be fairly easy.

There is unfortunately very little we can do until nodejs/node#18480 is merged – and I'm having a lot of trouble with operating systems that are not easily accessible to me 🙄

@gboysko
Copy link

gboysko commented Feb 15, 2018

I see. I'll see what I can do to restructure my application to use a single process. Thanks!

@gboysko
Copy link

gboysko commented Mar 8, 2018

@AndreasMadsen My application runs in several different clustering options (vertical, horizontal) as well as "none". When "none" is selected, a single process is used. However, in this configuration, I still get a failure. Note that my application still loads the cluster module, but it never "forks" a child process. Would you expect me to see the same JSON parsing error? Do I need to eradicate the cluster module from my source code to use this tool?

@AndreasMadsen
Copy link
Contributor

Note that my application still loads the cluster module, but it never "forks" a child process.

That should be fine.

/cc @mafintosh You could help solve this issue?

@AndreasMadsen AndreasMadsen reopened this Mar 8, 2018
@gboysko
Copy link

gboysko commented Mar 9, 2018 via email

@AndreasMadsen
Copy link
Contributor

AndreasMadsen commented Mar 9, 2018

@gboysko I think a good start would be:

clinic clean
clinic doctor -- node your-app-with-mode-none.js
ls -la

That way we can see if you are running other processes. We currently don't fully support that. The good news is that we will be able to support it in the next version of Node.js

@gboysko
Copy link

gboysko commented Mar 9, 2018

Hey @AndreasMadsen I ran again today and did not encounter a problem. I'm not sure how to explain why it worked, but I did let the server run without any load for several minutes after the stress test was done. I'm able to see the Doctor output now. Thanks!

@AndreasMadsen
Copy link
Contributor

Okay, very odd. The trace-events that this fails on, is an experimental node.js feature that still has some bugs, we are contributing fixes often. Let us know if the issue occurs again.

@AlexGochu
Copy link

AlexGochu commented Feb 19, 2023

Hi , guys.
Can you help me with same issue for

Windows 11
$ node -v v19.3.0
$ clinic -v v12.0.0

and as result of invoking

clinic doctor --on-port 'autocannon localhost:$PORT/api/review/byProduct/63f15234483137a0829dca29' -- node dist/main.js

getting result
image

PS: neither deleting node_module and lock files ( with consequencial reintalling all packages) nor changing port in my NestJS app does not bring any positive result?
Could you tell me what should I do to fix it on my side?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants