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

use fetch instead of puppeteer #141

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

liardoecp
Copy link
Contributor

@liardoecp liardoecp commented Oct 19, 2024

Hi everyone,

i was wondering if puppeteer is really necessary. I forked this repo and replaced puppeteer with fetch. It's seems works very well and the performance are better:

Fixes #121
Closes #138 #139 #145

Benchmark

Code

const run = async () => {
    const iterations = 10
    const responsesTime = []

    for (let index = 0; index < iterations; index++) {
        const start = new Date()
        await investing('1')
        responsesTime.push(new Date() - start)
        console.log(`run ${index + 1}: ${responsesTime[index]}ms`)
    }
    const avg = responsesTime.reduce((acc, responseTime) => acc + responseTime, 0) / responsesTime.length
    console.log(`avg: ${avg}ms`)
}

Results

With puppeteer

run 1: 1132ms
run 2: 3223ms
run 3: 1433ms
run 4: 1612ms
run 5: 1088ms
run 6: 1833ms
run 7: 1367ms
run 8: 1674ms
run 9: 1715ms
run 10: 1540ms
avg: 1661.7ms

With fetch

run 1: 371ms
run 2: 170ms
run 3: 139ms
run 4: 150ms
run 5: 138ms
run 6: 159ms
run 7: 170ms
run 8: 154ms
run 9: 164ms
run 10: 133ms
avg: 174.8ms

@DavideViolante
Copy link
Owner

Puppeteer was introduced because Investing website is protected by CloudFlare.
Related: #68 (comment)
I'm not sure if now it's working again without using puppeteer, I will also make some tests about it.

@liardoecp liardoecp force-pushed the remove-puppeteer branch 3 times, most recently from cd80a1c to 2690997 Compare October 21, 2024 17:49
@coveralls
Copy link

coveralls commented Oct 22, 2024

Pull Request Test Coverage Report for Build 11455888657

Details

  • 7 of 9 (77.78%) changed or added relevant lines in 1 file are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-13.7%) to 78.431%

Changes Missing Coverage Covered Lines Changed/Added Lines %
index.js 7 9 77.78%
Files with Coverage Reduction New Missed Lines %
index.js 4 78.0%
Totals Coverage Status
Change from base Build 11455326245: -13.7%
Covered Lines: 25
Relevant Lines: 32

💛 - Coveralls

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.

Max memory and restart windows server
3 participants