-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
tpl/data: Revise error handling in getJSON and getCSV #5199
Conversation
e182116
to
4290886
Compare
78607a2
to
470d8c4
Compare
9d850b6
to
21e5dfe
Compare
I just rebuilt hugo using this PR, but the build still fails on failing to fetch a remote JSON. To test if the build doesn't error out, I randomly replaced some chars in a valid API URL. But I got errors like these:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build still fails when a URL doesn't point to a JSON
Test snippet: {{ $works := getJSON "https://gitlab.com/api/v4/projects/kaushalmodi%2Fkaushalmodi.gitlab.io/repository/commits?path=content-org&ref_name=master" }}
{{ $failsUnauthorized := getJSON "https://gitlab.com/api/v5/projects/kaushalmodi%2Fkaushalmodi.gitlab.io/repository/commits?path=content-org&ref_name=master" }}
<!-- ^ replaced v4 with v5 -->
{{ $failsNotFound := getJSON "https://gitlab.com/api/v4/projects/foo%2Fkaushalmodi.gitlab.io/repository/commits?path=content-org&ref_name=master" }}
<!-- ^^^ replaced kaushalmodi with foo --> |
The most important part being: Log ERROR, but do not stop the build on remote errors. Fixes gohugoio#5076
You are right, I missed one. Fixe now. |
Thanks. Now hugo does not error out. But it still categorizes this as an Error, so it issues a non-zero exit code. This would fail Netlify build, as I see it failing my local Makefile based build too.
Can this be a Warning instead (I am assuming that warnings don't issue an error exit code)? |
No, it is definitively an ERROR. I thought we agreed on that? You are free to ignore the exit code if you want. The main motivation behind this particular PR is to ... not stop the build on such errors. |
The whole point of that issue I opened was to build sites like normal even if getJSON failed to retrieve anything from the URL. With the latest master, running
This functionally just like before because Makefile/Netlify would not proceed on seeing that error code (even if the
I cannot also unconditionally mask errors else I will reach the deploy step for real errors too. Can you please make this a warning? |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
The most important part being: Log ERROR, but do not stop the build on remote errors.
Fixes #5076