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

loadImage() does not handle exceptions properly, causing calling code to crash ("floating" async code) #865

Closed
lostfictions opened this issue Aug 9, 2024 · 0 comments · Fixed by #866

Comments

@lostfictions
Copy link
Contributor

lostfictions commented Aug 9, 2024

Hi, thanks for your work on this library!

I've been hitting an odd issue occasionally and it took me a while to diagnose. I have some code that looks like this...

try {
  const image = await loadImage(someUrl);
  // do some other things, call `ctx.drawImage()`, etc...
} catch (error) {
  console.error(error);
  // contextually handle the error...
}

However, occasionally this would cause errors that would "escape" the try-catch block, crashing the program. Something like this:

TypeError: Invalid URL
    at new URL (node:internal/url:797:36)
    at ClientRequest.<anonymous> (/[project folder]/node_modules/.pnpm/@napi-rs+canvas@0.1.44/node_modules/@napi-rs/canvas/load-image.js:68:30)
    at Object.onceWrapper (node:events:634:26)
    at ClientRequest.emit (node:events:519:28)
    at ClientRequest.emit (node:domain:488:12)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (node:_http_client:698:27)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17)
    at TLSSocket.socketOnData (node:_http_client:540:22)
    at TLSSocket.emit (node:events:519:28)
    at TLSSocket.emit (node:domain:488:12) {
  code: 'ERR_INVALID_URL',
  input: '/Images/somepicture.png'
}

I think there's two parts to this problem:

First, the reason this specific error arises is that sometimes redirects are relative and don't contain the full URL origin. So https://something.com/mypic.png might have a redirect to /pictures/mypic.png, which should be interpreted as https://something.com/pictures/mypic.png. But makeRequest() is constructing URLs without passing the base parameter, causing any relative redirects to throw an error on this line:

return makeRequest(new URL(res.headers.location), resolve, reject, redirectCount - 1, requestOptions)

However, there's also a second, broader problem: this error can't be caught by user code! It's located within a callback, and fails to catch the error and pass it to the reject() handler that makeRequest() receives. (In other words, it's sort of like a "floating promise", which is why none of the user's files appear within the stack trace above.)

There may be more types of errors within this kind of code that would similarly escape try...catch blocks (or Promise .catch() handlers) as well; I think the solution here is to add try...catch blocks in makeRequest().

lostfictions added a commit to lostfictions/canvas that referenced this issue Aug 9, 2024
lostfictions added a commit to lostfictions/canvas that referenced this issue Aug 9, 2024
lostfictions added a commit to lostfictions/why-cant-i that referenced this issue Aug 9, 2024
Brooooooklyn added a commit that referenced this issue Aug 14, 2024
…o loadImage() (#866)

* fix: handle invalid urls in load-image.js (#865)

* fix: handle relative urls in redirects (#865)

---------

Co-authored-by: LongYinan <lynweklm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant