getJSON fetch failure should not abort build

[anthonyfok]

Hugo >= 0.50 (including master HEAD as of today) exits before finishing building a website when getJSON fails to get a resource.

This problem was earlier filed as Issue #5076 "getJSON etc. should log ERROR but return nil in failure situations" and was fixed in 0.49, but perhaps the "greatly improved error messages shown in browser" feature in 0.50 might have undone the fix in PR #5199.

[anthonyfok]

git bisect v0.50 v0.49 says that d1661b8 is the first bad commit. Investigation continues.

[bep]

Not sure what finding how finding who/what broke it will help fixing it. Why not just fix it?

[anthonyfok]

I am actually trying to fix it; I am just documenting the progress along the way by pasting the result of the last "git bisect" command, though probably the output of that command was too verbose. I'll edit my last message to remove the details and just keep the commit hash to keep track of it.

[anthonyfok]

Trying to find clues from the messages shown by hugo -v...

Before:

ERROR 2019/01/27 16:16:23 Failed to get JSON resource https://api.example.com/product?name=foobar: Failed to retrieve remote file: Not Found
Total in 1234 ms
Error: Error building site: logged 1 error(s)

After:

Error: Error building site: failed to render pages: [en] page "Test Website": render of "home" failed: execute of template failed: template: index.html:24:13: executing "main" at <getJSON "https://api...>: error calling getJSON: failed to get getJSON resource "https://api.example.com/product?name=foobar": Failed to retrieve remote file: Not Found

[anthonyfok]

My first attempt at a fix is at PR #5648, though I am starting to think I tried to fix the problem at the wrong place, and at a cost too: The nice context information provided by _errors.Wrapf() (from pkg/errors) is lost. So, simply replacing with _errors.Wrapf() with ns.deps.Log.ERROR.Printf() reverting to the pre-0.50 behaviour of no error context isn't ideal.

I am thinking that there should be a way to distinguish between fatal and non-fatal errors, maybe with the non-fatal ones marked with a "[non-fatal]" suffix in the error string so it can be checked later. (I have just barely started looking at the error handling code, so what I just said may be utter nonsense, my apologies.) Unfortunately, I won't have time to look at this issue again in this week or the next, hence this note in case anyone else (or maybe the future "me") wants to pick it up.

[Lnaden]

I'd like to revive this issue as failures in getJSON still cause builds to fail and halt in the versions 0.68.3, 0.69.1 and 0.69.2 which I have tested it on.

The use case I have is that we're checking a database through its REST API for the existence of an entry to pull some metadata, and falling back to local data if not present. However, the only way we can check for the existence is to make a query, and if it's not there, the getJSON errors, and the build halts.

Specific error is this: Failed to get JSON resource "https://data.rcsb.org/rest/v1/core/entry/": Failed to retrieve remote file: Not Found.

Supposedly #5648 was meant to fix this, but from what I'm reading never did it properly?

I don't know Go so I'm not sure exactly how to fix it, but I think I can trace its stack:

• getJson calls getResource and then if an error is thrown its supposed to log said error but not throw the error:

  hugo/tpl/data/data.go

  Lines 118 to 122 in c2d9fd1

  	err = ns.getResource(cache, unmarshal, req)
  	if err != nil {
  	ns.deps.Log.ERROR.Printf("Failed to get JSON resource %q: %s", url, err)
  	return nil, nil
  	}

• getResource attempts to call getRemote because the a URL was passed in, so the "" case is skipped and it goes to the default:

  hugo/tpl/data/resources.go

  Lines 109 to 122 in c2d9fd1

  	switch req.URL.Scheme {
  	case "":
  	url, err := url.QueryUnescape(req.URL.String())
  	if err != nil {
  	return err
  	}
  	b, err := getLocal(url, ns.deps.Fs.Source, ns.deps.Cfg)
  	if err != nil {
  	return err
  	}
  	_, err = unmarshal(b)
  	return err
  	default:
  	return ns.getRemote(cache, unmarshal, req)

• getRemote detects an HTTPError and actually throws the error, which passes back up the stack:

  hugo/tpl/data/resources.go

  Lines 55 to 57 in c2d9fd1

  	if isHTTPError(res) {
  	return nil, errors.Errorf("Failed to retrieve remote file: %s", http.StatusText(res.StatusCode))
  	}

• getResource and getJSON behave correctly and log the error, but the thing actually halting the build came from the getRemote call and the HTTPError handling.

I have a couple ideas on what behavior I would like to see to make this more user and use-case friendly.

1. Is it possible to make the getRemote on HTTPError not actually throw the error and instead log it, pass it back up and let the calling function handle it? Especially since getRemote isn't a publicly available function in Hugo's shortcodes?
2. Could there be some switch to tell the getRemote to pass it back up (like if there is another function invoking which could handle it?)
3. Could the getRemote or getJson throw a warning instead? This does leave it up to the site developer to properly handle the case when the JSON they get back isn't what they expect, but I think that's a reasonable thing to ask for.
4. Use a with...else...end construct to handle the return from getJSON when it errors. This actually already works as expected! I can test it with the shortcode construct of

   {{ with getJson "...." }}
       <!-- Do stuff on success -->
   {{ else }}
       {{ warnf "It failed" }}
   {{ end }}
   

   This will correctly throw not only the getJSON error, but also throw the warnf warning in the console. However, the fact that an error was thrown causes the built to fail in the end, but the getJSON error clearly is not fatal to the overall build code.
5. Have a function to probe an external site? Basically a function to see if the url exists. I think getRemote does this already to some extent, but make this return a boolean instead of the result or error?

[pgorod]

If I may just add my use case, to explain why this is important to me. This is from a forums thread a while ago:

An example just to try to get you to sympathize: please see this page on our site. Scroll down until you see the Github avatars of the contributors. That’s coming form the Github API.

So, one day it’s working. A few months later, after some totally unrelated change in content, the Netlify build is failing because of the getJSON call. The cause? One of the contributors deleted his account on Github, and the call to get his Avatar fails.

Obviously, I should just be able to put in a dummy placeholder Avatar and get on with my life. An API is for consuming dynamic information. A query that returns 0 results is not necessarily an application error - it can be a normal every day occurrence, and we need to be able to handle it.

@Lnaden thanks for reviving this and for your thoughts on the subject.

[anthonyfok]

Hi @Lnaden,

Thank you for your report. However, since this issue was already fixed on v0.54,
what you see may be similar manifestation of a different cause in later version.

So, instead of "reviving" an old fixed issue, I think it would be better to open up a new issue that mentions this old issue.

Thanks!

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.