Update index.js to provide correct URL (fixes issue #2)

[cfepdan]

Reopened pull request with updated commit.

[cfepdan]

I have tested it. I don't have any automated testing experience, but I can create a little test package that works in Apify cloud. Will that help?

On Thu, Jan 27, 2022, 11:24 AM Ondra Urban @.***> wrote:

@.**** requested changes on this pull request.

I don't remember much from the days I wrote this, but I think there was some issue with pulling the frontend like this back then. That's why I needed this magical URL in the first place. But things may have changed for sure.

So the question is, have you tested it? Can we add a test for it?

In src/index.js https://github.com/apify/devtools-server/pull/4#discussion_r793780215:

@@ -145,7 +145,7 @@ class DevToolsServer {

     // http://localhost:9222/devtools/inspector.html?ws=localhost:9222/devtools/page/0BAC623431B93A0908551626AA14247D
     const correctDevtoolsUrl = devtoolsUrl.replace(`ws=localhost:${this.chromePort}`, `${this.wsProtocol}=${this.containerHost}`);

• return https://chrome-devtools-frontend.appspot.com/serve_file/@${hash}/${correctDevtoolsUrl}&remoteFrontend=true https://chrome-devtools-frontend.appspot.com/serve_file/@$%7Bhash%7D/$%7BcorrectDevtoolsUrl%7D&remoteFrontend=true;
• return https://${this.containerHost}/devtools/${correctDevtoolsUrl}&remoteFrontend=true;

So we don't need the hash anymore. Would be good to remove it on line 144 so that we don't have an unused variable here.

— Reply to this email directly, view it on GitHub https://github.com/apify/devtools-server/pull/4#pullrequestreview-865117769, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGWGAJLDB5G4526ZUJREAW3UYFWVHANCNFSM5M4474YQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you modified the open/close state.Message ID: @.***>

[mnmkng]

I trust you :) Please remove the unused hash variable and I'll merge it.

[cfepdan]

@mnmkng I've added a new function, fetchDevToolsUrl() that returns just the URL without the hash. I did not remove the old function that returns both--presumably the appspot.com format worked/could work with some tweaking... that said, I've tested this, and it works in my Apify actor.

[mnmkng]

Cool, let's see what it does. Thanks!

[mnmkng]

@cfepdan Before release I wanted to test that it works with the latest version of Puppeteer and the automated test that you can run with npm test failed for me. Could you please see why? You have much better insight into the problem now. Thanks! I'm reluctant to release the package with a failing test, not knowing what causes the failure.