getURLVideoID error behaviour

[nfearnley]

Why does getURLVideoID() return an error instead of throwing an error? This is completely unintuitive, and is not documented.

[issue-label-bot]

Issue Label Bot is not confident enough to auto-label this issue. See dashboard for more details.

[fent]

i think it used to throw before, and had people before ask why it throws, since throwing would break their node app if they hadn't wrapped it in a try/catch block.

ideally it would return null if it can't find the video id. but the error it returns are specific on why it wasn't able to find the video id: https://github.com/fent/node-ytdl-core/blob/master/lib/util.js#L272-L290

maybe it could work like

[id, err] = getUrlVideoID(url);

which is a pattern common in golang

[nfearnley]

Generally, the convention I'd expect would be this: Synchronous calls: Return value or throw error Return value or return null Return value or return undefined Callbacks: Call callback with (err, value) Promises: Resolve with value or reject with err Right now, by returning an error, I need to add the following to my calls to prevent it from crashing: var value = ytdl.getURLVideoID(input); if (value instanceof Error) { // handle error } This isn't much different from a try catch, other than it's against convention.

…

On Mon, Sep 9, 2019, 1:10 AM fent ***@***.***> wrote: i think it used to throw before, and had people before ask why it throws, since throwing would break their node app if they hadn't wrapped it in a try/catch block. ideally it would return null if it can't find the video id. but the error it returns are specific on why it wasn't able to find the video id: https://github.com/fent/node-ytdl-core/blob/master/lib/util.js#L272-L290 maybe it could work like [id, err] = getUrlVideoID(url); which is a pattern common in golang — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#485?email_source=notifications&email_token=AAEJHAQ5FRP4XGYEI5H3ZTLQIXLDJA5CNFSM4IUVM7RKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6GIDPI#issuecomment-529301949>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJHAW5N7KNVAEJ4LZ6OP3QIXLDJANCNFSM4IUVM7RA> .

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[TimeForANinja]

<3 git blame
#121

[fent]

@nfearnley there's a PR for a solution to this now #520

implementation looks good to me, what do you think?