Skip to content

Conversation

@polikeiji
Copy link

@polikeiji polikeiji commented May 3, 2021

RSS is always returned as NaN, and I think it causes that the "mainProcessBytes" in the response of "Apify.getMemoryInfo" is always returned as "NaN" on Windows environment.

Copy link
Member

@jancurn jancurn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for your fix! Please see a small comment

index.js Outdated
if (process.platform == 'win32') {
// For Windows, WMIC.exe never returns any value for "Status" and it causes "WorkingSetSize" is set at columns[3]
// See: https://docs.microsoft.com/ja-jp/windows/win32/cimwin32prov/win32-process?redirectedfrom=MSDN
columns[4] = parseInt(columns[3], 10);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be safer to just skip "Status" in the WMIC call above. In case WMIC shows some non-empty string for it, this code here will fail.

@polikeiji
Copy link
Author

@pocesar
Thank you for your comment! I updated it. Please review them when you have time.

@polikeiji polikeiji requested a review from jancurn May 3, 2021 14:55
@jancurn jancurn requested a review from pocesar May 4, 2021 11:28
Copy link
Member

@jancurn jancurn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jancurn jancurn merged commit 9878362 into apify:master May 4, 2021
@jancurn
Copy link
Member

jancurn commented May 4, 2021

Merged and published, will get to Apify SDK soon - apify/crawlee#1017

@polikeiji polikeiji deleted the bug/rss-is-nan-on-windows branch May 4, 2021 13:00
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

Successfully merging this pull request may close these issues.

3 participants