microsoft / ApplicationInsights-node.js

Microsoft Application Insights SDK for Node.js
MIT License
320 stars 138 forks source link

EEXIST error when using functions_worker_process_count #1230

Closed ejizba closed 1 month ago

ejizba commented 8 months ago

In Azure Functions, users can have multiple processes on the same instance if they use the functions_worker_process_count setting (docs here). Unfortunately that can lead to the following uncaught exception:

Error while saving data to disk: [object Error]{ stack: 'Error: EEXIST: file already exists, mkdir 'C:\local\Temp\appInsights-node...

I don't know the best behavior here, but at minimum this error should be handled more gracefully instead of throwing an uncaught exception which can crash the Node.js process.

JacksonWeber commented 2 months ago

@ejizba Just wanted to confirm with you that the solution implemented in the above PRs works for you. Thanks.

ejizba commented 1 month ago

@JacksonWeber this was a customer incident and I don't know exactly how to repro this issue myself. I tried on an old version of the appinsights package but I didn't see the error. Do have any tips for how to hit this code path? When is that file used?

JacksonWeber commented 1 month ago

@ejizba IIRC the only way I could figure out to create the situation required to reproduce this problem was to create an Azure Function with access to multiple threads, and then have a sufficiently resource intensive process run on that function. I wasn't able to reproduce, but the logic behind the PR was to create a naming convention for created cache files that utilizes the PID such that no two processes could attempt to operate on the same file at once, even in the situation the customer described.

ejizba commented 1 month ago

Okay I reviewed the PRs and they look good to me. This was only a single incident and it was arguably not even their main problem (high CPU). I think it's safe to close for now and we can just open a new issue if we ever see it happen again.