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

Prevent exception when passing directory instead of file #3446

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jmorrell-cloudflare
Copy link
Collaborator

@jmorrell-cloudflare jmorrell-cloudflare commented Jan 31, 2025

If you accidentally pass a directory path instead of a capnp config file to workerd serve $path you get an uncaught exception:

*** Uncaught exception ***
kj/filesystem-disk-unix.c++:365: failed: mmap: Invalid argument

This change adds an assertion that the provided path is a file type. Invoking workerd serve with a directory now results in the following error:

> workerd serve samples/tail-workers/
workerd serve: samples/tail-workers/: Config path is not a file.
workerd serve --help' for more information.

@jmorrell-cloudflare jmorrell-cloudflare requested review from a team as code owners January 31, 2025 21:30
@jmorrell-cloudflare jmorrell-cloudflare changed the title Prevent uncaught exception when passing directory instead of file Prevent exception when passing directory instead of file Jan 31, 2025
Copy link

github-actions bot commented Jan 31, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@jmorrell-cloudflare
Copy link
Collaborator Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Jan 31, 2025
Copy link

github-actions bot commented Jan 31, 2025

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@kentonv
Copy link
Member

kentonv commented Jan 31, 2025

Hmm, the code looks right to me but CI suggests it is not, somehow.

@danlapid
Copy link
Collaborator

danlapid commented Feb 3, 2025

Not sure I understand what's going on here, perhaps a rebase will solve it? 🤔

@jmorrell-cloudflare jmorrell-cloudflare force-pushed the jmorrell/fix-segfault-on-directory branch from 234d182 to 8377a11 Compare February 3, 2025 18:31
@jmorrell-cloudflare jmorrell-cloudflare force-pushed the jmorrell/fix-segfault-on-directory branch from aed936d to 82c60e3 Compare February 7, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants