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

Configurable error handling for prerender #1940

Closed
happycollision opened this issue Jul 18, 2021 · 4 comments · Fixed by #2007
Closed

Configurable error handling for prerender #1940

happycollision opened this issue Jul 18, 2021 · 4 comments · Fixed by #2007
Labels
feature request New feature or request help wanted PRs welcomed. The implementation details are unlikely to cause debate

Comments

@happycollision
Copy link
Contributor

happycollision commented Jul 18, 2021

Describe the problem

You can reference #1935 for some context. I was pretty frustrated that my build was failing on the very first 404 it encountered once the adapter took over. I had no idea that there was an option to change that behavior. Yes, I eventually found it, but I looked several times in the wrong location.

There are several reasons I didn't find the option I was looking for:

  1. I assumed that since the failure was during the adapter phase of the build that I should be looking at the adapter instead of the kit config.
  2. I had no idea that the failure was happening during prerender, because I just wasn't familiar enough with the steps involved with building.
    1. And --verbose does not indicate you are prerendering either
    2. Upon closer inspection of the stack trace, "prerender" appears, but so does "adapt" and "visit", with "visit" being the most prominent in reference to the error.
  3. prerender.force seems—to me—to suggest an override, not of a failure, but of some other option that perhaps a plugin is trying to set or something.

When initially looking through the docs, I saw the section describing prerender options. I even read crawl and enabled carefully but completely skipped over force for the reason I mentioned above.

Eventually I thought to search for the word "fail" in the docs (luckily only 6 occurrences) and found that the description I glanced right past had the answer.

Yes, It is in the docs. Yes, it clearly states what the option does. But its name threw me off the trail.

Describe the proposed solution

More log information

I think that, in general, more indication of what is happening during the build might have led me to pay closer attention to the option group I needed. If there was more indication of what was happening in the terminal, perhaps I'd have seen that, "Oh, we are in the prerendering phase" when it failed.

I really thought that the adapter was totally separate from the config for the rest of the app, but apparently not!

Different naming

In addition to that, a different name for the force option might be better. One of these?

  • continueOnError
  • continueOn404
  • bailOnError (inverse)
  • bailOn404 (inverse)

Bonus suggestions: option interface changes

Many toggles

Perhaps—if someone wants to get crazy—more granular control? I assume that there are other errors besides a missing image link that can fail a prerender. It might be nice if you could specify...

{
  prerender: {
    failBuildOn: {
      missingMediaLink: false,
      missingInternalPageLink: true,
      missingExternalPageLink: false,
      // ... etc
    },
  },
}

But I am guessing that is WAY overboard. And probably presents a fairly large surface area for bugs.

Escape hatch

Though you could instead do something like this

interface KitOptions {
  prerender: {
    onError: 'fail' | 'continue' | (err: Reason) => boolean
    // ... snip
  }
}

Incidentally, seeing onError: 'fail' in the docs would certainly give a better impression of what is happening than force: false, and would accomplish something similar to a general renaming of the option.

Alternatives considered

Do nothing and tell folks to RTFM. 🤷‍♂️ 😉

Add something to a troubleshooting section?

Add a specific message when a build fails in the prerender phase:

> 404 /g/images/perennials/tom-signature.png (linked from /news/2020-03-25-season-cancelled)
>>> Failing the build. See `prerender` options to continue on errors in the future.

Importance

nice to have

Additional Information

No response

@benmccann
Copy link
Member

If we were going to change it, I think something along the lines of onError: 'fail' | 'continue' | (err: Reason) => boolean would be the best since it's most configurable. It'd be a pretty easy change to make:

const error = config.kit.prerender.force

@benmccann benmccann added feature request New feature or request help wanted PRs welcomed. The implementation details are unlikely to cause debate labels Jul 20, 2021
@benmccann benmccann changed the title Help people find prerender.force, or rename it Configurable error handling for prerender Jul 20, 2021
happycollision added a commit to happycollision/kit that referenced this issue Jul 24, 2021
@happycollision
Copy link
Contributor Author

Did some preliminary work on this in the branch referenced just above. I'll open a PR soon, after docs changes are added.

happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes @sveltejs/kitsveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 25, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
@happycollision
Copy link
Contributor Author

Done. #2007 is ready for review.

happycollision added a commit to happycollision/kit that referenced this issue Jul 26, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 27, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
happycollision added a commit to happycollision/kit that referenced this issue Jul 27, 2021
fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
@janosh
Copy link
Contributor

janosh commented Jul 27, 2021

Thanks @happycollision, great work! Had the same problem so fully agree that prerender.force wasn't the most helpful name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request help wanted PRs welcomed. The implementation details are unlikely to cause debate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants