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

feat: improve resilience of the parser #6

Merged
merged 14 commits into from
Nov 26, 2024
Merged

feat: improve resilience of the parser #6

merged 14 commits into from
Nov 26, 2024

Conversation

hydrobeam
Copy link
Owner

Goal

Make it so that org-rust never panics and instead does what it's supposed to do / provide a reasonable user-facing error.

A lot of these cases were due to improper EOF handling in the parser, but there were other more general errors that should not have been happening.

contents

The commits describe the changes.

tldr: A lot of bug fixes and more tests to make sure these issues don't happen again

no more errors on EOFS

handle attr
the goal was to allow

* EOF

to be interpeted as a valid headline.this one was involved so i'll
provide some details:

1. extract title parsing to an external function so we can freely error
in case a title isnt provided
2. never panic on EOF, this was happening in two places
3. make the tests actually.. test

add make_node_id method for testing
nothing breaking, i just updated the formatting a while ago and never
got around to resolving the errors it introduced in the tests
include and macro handling in the export error now expose real errors,
not just yolo panicking and unwraps.
no longer unwraps, instead passes the error on screen

also clean them up a bit for presentation
Copy link

cloudflare-workers-and-pages bot commented Sep 22, 2024

Deploying org-rust with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3ac8e91
Status: ✅  Deploy successful!
Preview URL: https://82455aa0.org-rust.pages.dev
Branch Preview URL: https://resillience.org-rust.pages.dev

View logs

no longer stops after just one. also update org-wasm/org-cli to report
these to the user.

had to update the tests since there isnt a clean way to call =?= on a
vec of items, so they all return nothing (and unwrap) instead of
Result<()>. probably cleaner this way anyways
since we now coalesce errors and don't use the ? syntax, the only other
source of those were in write!() calls. we almost exclusively write! to
in memory string buffers (or maybe stdout), so I expect write! to never
fail (at least not without the entire program catastrophically failing
right alongside it)

aside:
I guess someone could pass in some network thing as a buffer where
failure is more likely but it's not like we're recovering from a failure
anyways.

all that being said, I decided to use unwrap to just have the program
explode if write! fails. it's more appropriate than failing silently
since clearly something disastrous has happened and a panic would be
what I want in that scenario. it feels icky to cause a panic but I think
it's what a programmer would want in that scenario (and it'll almost
never happen)
Copy link

github-actions bot commented Nov 26, 2024

Deploy Preview ready!

Name Link
Latest commit c167306
Latest deploy log https://github.com/hydrobeam/org-rust/actions/runs/12037091434
Deploy Preview Url

@hydrobeam hydrobeam merged commit c167306 into main Nov 26, 2024
1 check passed
@hydrobeam hydrobeam deleted the resillience branch November 26, 2024 18:47
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.

1 participant