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

Update getWatchHTMLPage right pos #946

Merged
merged 2 commits into from
Jun 26, 2021
Merged

Conversation

mome0320
Copy link
Contributor

@mome0320 mome0320 commented Jun 23, 2021

in WatchHTMLPage
info.response isn't end \n (it's last line)
so update right pos </script> is better.

In test I think it works.
image

Copy link

@RisedSky RisedSky left a comment

Choose a reason for hiding this comment

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

This fix the issue using ytdl-core for Discord music bot
see #946 (review)

@TimeForANinja
Copy link
Collaborator

🤔 can't replicate
can you provide me with a watchHTMLPage you receive?
a simple fs.writeFileSync befor the line you edited should do it

Copy link

@RisedSky RisedSky left a comment

Choose a reason for hiding this comment

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

This doesn't fix, getting the following error after playing the song :

stack: 'Error: aborted\n at connResetException (node:internal/errors:683:14)\n at TLSSocket.socketCloseListener (node:_http_client:407:19)\n at TLSSocket.emit (node:events:406:35)\n at node:net:661:12\n at TCP.done (node:_tls_wrap:580:7)\n at TCP.callbackTrampoline (node:internal/async_hooks:130:17)'

@mome0320
Copy link
Contributor Author

can't replicate
can you provide me with a watchHTMLPage you receive?
a simple fs.writeFileSync befor the line you edited should do it

https://gist.github.com/mome0320/5ef235f47239a2dcb90f35baa79daa3c
here this file.

@mome0320
Copy link
Contributor Author

mome0320 commented Jun 23, 2021

This doesn't fix, getting the following error after playing the song :

stack: 'Error: aborted\n at connResetException (node:internal/errors:683:14)\n at TLSSocket.socketCloseListener (node:_http_client:407:19)\n at TLSSocket.emit (node:events:406:35)\n at node:net:661:12\n at TCP.done (node:_tls_wrap:580:7)\n at TCP.callbackTrampoline (node:internal/async_hooks:130:17)'

this PR may not affect about this error.

@TimeForANinja
Copy link
Collaborator

any reason for not moving all three findJSON to '</script>' ?
besides that this pr looks fine - already thanks for the time 😉

@mome0320
Copy link
Contributor Author

any reason for not moving all three findJSON to '</script>' ?
besides that this pr looks fine - already thanks for the time

in 318 line?

  try {
    info.player_response = findJSON('watch.html', 'player_response',
      body, /\bytInitialPlayerResponse\s*=\s*\{/i, '\n', '{'); // this?
  } // ...

It works even if you don't change this line
but move all end '</script>' is fine for me.

@TimeForANinja
Copy link
Collaborator

TimeForANinja commented Jun 24, 2021

yep, exactly that code

but move all end '</script>' is fine for me.

let's do it then
don't wanna touch this code again in the next months 😂

@TimeForANinja
Copy link
Collaborator

nice - tests are passing
gonna wait for #938 to be ready to merge before merging this...
will only confuse ppl when we bring half an update now...

@github-actions
Copy link

🎉 This PR is included in version 4.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@TimeForANinja
Copy link
Collaborator

Thanks again for the PR @mome0320 and everyone else who helped
For further Problems with (especially) video details check out #958

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants