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

Error stack in worker missing #4728

Closed
Fenzland opened this issue Apr 13, 2020 · 6 comments · Fixed by #7987
Closed

Error stack in worker missing #4728

Fenzland opened this issue Apr 13, 2020 · 6 comments · Fixed by #7987
Assignees
Labels
bug Something isn't working correctly cli related to cli/ dir web related to Web APIs

Comments

@Fenzland
Copy link
Contributor

Currently, every error thrown in worker has no trusted stacks, but only

at WorkerImpl.#poll ($deno$/web/workers.ts:140:17)
@bartlomieju bartlomieju added bug Something isn't working correctly cli related to cli/ dir web related to Web APIs labels May 21, 2020
@bartlomieju bartlomieju changed the title Error stack in worker Error stack in worker missing May 21, 2020
@bartlomieju bartlomieju self-assigned this May 21, 2020
@bartlomieju
Copy link
Member

@nayeemrmn could you look into this issue? It would require to serialize the error in Rust, forward it to JS (worker host) and then properly reconstruct it

@nayeemrmn
Copy link
Collaborator

Based on browser behaviour, the worker should print errors to the terminal on its own. The parent worker is only made aware of the error if the user sets an error event listener on the worker object. I'll see what I can do.

@bartlomieju
Copy link
Member

Based on browser behaviour, the worker should print errors to the terminal on its own. The parent worker is only made aware of the error if the user sets an error event listener on the worker object. I'll see what I can do.

@nayeemrmn I think that's what happenning right now - if there's no error listener in the host then error is rethrown in host context. Thanks for looking into this 🙏

@nayeemrmn
Copy link
Collaborator

if there's no error listener in the host then error is rethrown in host context.

Okay, I think uncaught errors in Workers should print to the terminal themselves, and as a Deno-specific extension to satisfy our design goal, the main worker should terminate upon polling an unhandled error event like it does now. Since the Worker error would be printed above it, the error thrown by the main worker can stay vague.

error: Uncaught (in worker "worker-name") NotFound: No such file or directory (os error 2)
    at Object.jsonOpSync (core.js:247:13)
    at Object.openSync (deno:cli/rt/30_files.js:30:22)
    at file:///mnt/c/Users/Nayeem/projects/deno/temp-worker.js:3:8
error: Uncaught Error: Unhandled error in worker "worker-name".
    at Worker.#poll (deno:cli/rt/11_workers.js:157:19)

@bartlomieju
Copy link
Member

@nayeemrmn I think that's not entirely correct - according to spec if there's no error handler attached to Worker then error should be propagated to the scope the worker was defined in - so send error event to the window

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented Oct 7, 2020

It seems the event should travel upwards, but I'm saying only the source Worker where the error occurred needs to be responsible for printing the genuine error stack (if it doesn't have a handler itself).

I'm proposing that if the source worker has its own error handler which prevents default, nothing is printed. If it doesn't, it prints the error stack to stdout and propagates an event error upwards. If the event error reaches the main worker without being handled, we would just let it error out in Worker.#poll() like it does now and cause termination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly cli related to cli/ dir web related to Web APIs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants