MindscapeHQ / raygun4node

Node.JS SDK for Raygun
https://raygun.com
MIT License
31 stars 29 forks source link

Question: any way to abort long-running `send` call? #311

Closed markcellus closed 1 month ago

markcellus commented 1 month ago

This isn't a bug or feature request, but opening issue as a question.

I'm noticing that occasionally there could be a while before Raygun's send call responds and holds up connections that can be used for other sources on my server. As a result, whenever there are many send calls, my server takes quite a hit and slows down. In these cases, is it possible to set a timeout for the send call?

TheRealAgentK commented 1 month ago

Hi @markcellus - thanks for your question.

Generally, the Raygun back end should respond reasonably quickly - if you find you have regular issues with long send time, I'd encourage you to maybe have a chat to Raygun's customer service team as well to have that looked into.

But re the provider, you're actually correct, I think we missed setting a timeout on the send-calls. Would a general timeout of x seconds be what you'd need or would you prefer to be able to customize and override it with your own value?

markcellus commented 1 month ago

Ok thank you. Yes being able to customize with my own value would be great.

However, I'd like to be able to use an AbortSignal.timeout() or similar so that I can control the behavior when/if it times out.

TheRealAgentK commented 1 month ago

Ok, I'll have a chat with the other folks in the team and we'll see what we can do. Thanks for the feedback, I'll keep you up to date!

TheRealAgentK commented 1 month ago

Sorry @markcellus - just another question: Are you using batched transport (https://github.com/MindscapeHQ/raygun4node?tab=readme-ov-file#batched-error-transport) or are you just sending individual crash reports?

markcellus commented 1 month ago

Thanks for the quick reply! No batchFrequency is set, but we do have batch set to true.

TheRealAgentK commented 1 month ago

@markcellus Ok, that's interesting. When you use batching, the requests get enqueued, but the Promise doesn't get completed until it is fully sent. If you don't use the batch functionality, do you still see these kind of issues?

We're adding configuring timeouts soon, but I'm not sure if the route of AbortSignal or similar ideas to cancel the promise will happen in the short term. We're going to have a look into that, but it introduces certain architectural problems that we need to look at.

markcellus commented 1 month ago

Understood! Thank you for at least looking at this. I've been pretty busy with work these days, but I'll see if I can reproduce without the batching. I don't think we've upgraded to the version where send is a promise yet, btw. Maybe I can try that as well.

TheRealAgentK commented 1 month ago

There is a PR that I've started (https://github.com/MindscapeHQ/raygun4node/pull/320) and just need to tweak and document later today that will deliver a timeout solution. So, you could wait a couple of days until that's released and give that a go.

miquelbeltran commented 1 month ago

Hi @markcellus we have published this feature in the version 2.1.0-alpha available in the alpha tag.

You can install it by running npm install raygun@alpha and it should add 2.1.0-alpha to your project.

markcellus commented 1 month ago

Thank you! Aiming to try this out this week. Will let you know

TheRealAgentK commented 3 weeks ago

@markcellus Did you have a chance to give it a try? Would be cool to get some feedback from you before we take it out of the alpha tag.

markcellus commented 3 weeks ago

Hey @TheRealAgentK thanks for adding this. I took a look at the PR and the code change, which look ok. But I'm not quite sure how I'd be able to test this unless we can isolate the issue and make it reproducible. The issue is random and happens to only be in our production environment (can't produce in lower ones). I'm also hesitant to pull an alpha version of 2 into the project for prod because we haven't even upgraded to v2 yet. :)

I'd say if you think it works and it's well tested, feel free to add it to your release. I'll try to get us upgraded to v2 (test that) and then we can give the timeout a go in a higher stable version.

Thanks again!

TheRealAgentK commented 2 weeks ago

@markcellus We published 2.1 yesterday, let us know how you go!

Thanks so much for your patience and ongoing feedback, really appreciated!