CSS Spec Preprocessor keeps failing

[annevk]

But fetching https://api.csswg.org/bikeshed/ myself and using it via WHATWG CI works fine. I've seen this for a number of days now and it seems like the error might be with PR Preview.

[tobie]

I’m not sure I understand the issue, here. Would you pointers to the pull requests experiencing that issue?

[annevk]

whatwg/storage#97
whatwg/storage#93
whatwg/xhr#279
whatwg/fetch#1029

[annevk]

Outside WHATWG: w3c/webappsec-secure-contexts#77

[tobie]

Hey, @plinss & @tabatkins, would love your help here:

I'm not super sure what's going on.

All of the above specs are failing to build atm.

Navigating to the following returns a 404:

https://api.csswg.org/bikeshed/?url=https%3A%2F%2Fraw.neting.cc%2Fw3c%2Fwebappsec-secure-contexts%2Fb8e55d93db57e9b53ca9ca23eb8d2fe3093b6d85%2Findex.src.html&force=1&md-warning=not%20ready

Running the unescaped URL (https://raw.githubusercontent.com/w3c/webappsec-secure-contexts/b8e55d93db57e9b53ca9ca23eb8d2fe3093b6d85/index.src.html) from the HTML interface at https://api.csswg.org/bikeshed/ yields the following error message regardless of the "Die On" drop-down setting:

Traceback (most recent call last):
  File "/sites/api.csswg.org/bikeshed/bikeshed.py", line 6, in 
    bikeshed.main()
  File "/sites/api.csswg.org/bikeshed/bikeshed/cli.py", line 220, in main
    handleSpec(options, extras)
  File "/sites/api.csswg.org/bikeshed/bikeshed/cli.py", line 256, in handleSpec
    doc.preprocess()
  File "/sites/api.csswg.org/bikeshed/bikeshed/Spec.py", line 109, in preprocess
    self.assembleDocument()
  File "/sites/api.csswg.org/bikeshed/bikeshed/Spec.py", line 133, in assembleDocument
    self.md.computeImplicitMetadata(doc=self)
  File "/sites/api.csswg.org/bikeshed/bikeshed/metadata.py", line 175, in computeImplicitMetadata
    if self.repository.type == "github" and "feedback-header" in self.boilerplate and "repository-issue-tracking" in self.boilerplate:
AttributeError: 'NoneType' object has no attribute 'type'

It seems there's a couple of issues happening at the same time:

1. There's a new unrecognized type NoneType that Bikeshed is chocking on.
2. CSS PreProcessor is no longer recognizing the "Die On" setting passed either through the interface or through the URL query params

[plinss]

api.csswg.org is returning a 400, I think that's because bikeshed is crashing.

I believe this is something @tabatkins needs to look at. Bikeshed should not be crashing.

[tabatkins]

Okay, looks like in the refactoring for URL-based inputs, the code to figure out what repository the spec is in missed a branch and starting returning None in some cases. That's fixed. Quite surprised it didn't trigger during earlier testing, tho; maybe all the specs we were testing on had a Repository metadata already?

Anyway, fix pushed.

[annevk]

@tobie perhaps you could distinguish between api.csswg.org being down and returning a 400? That might make it clearer where to go first next time. Also happy to try contribute a patch for that.

[tobie]

Agree that this error code is confusing. CSS Spec Preprocessor should really be returning a 500 here. I’d rather the error code gets fixed there, if that’s at all possible, than start special casing http codes for different services. Sort of defeats the purpose of standardizing http codes, imho.

That said, I’m happy for suggestions on how to surface those errors more clearly to you all.

[plinss]

Agree it should return a 500 if Bikeshed crashes, and a 400 if Bikeshed exits with an error. I’ll look in to that, it may be that I’m getting the same exit code from Bikeshed either way. Might have to patch Bikeshed...

[annevk]

Oh, I totally missed that it already says "Error: 400 Bad Request". Somehow I read that as it being a network error. I'm not sure there's much more to do here for PR Preview.

I can also confirm that Tab fixed it.

[tobie]

Another thing that would be really useful to surface would be Bikeshed’s error itself.

W3C’s rendering service returns a JSON object back with an error field that contains ReSpec’s errors when it fails to build. Similarly Wattsi returns a log dump. PR Preview handles both of those and surfaces the error in the pull request.

@plinss is there anyway you could securely pass a stack trace, a formatted error message, or both in the body of the response?

[tobie]

This is so much more useful already!

Love it.

Update: see @plinss's comment below. This is the existing behavior when you're not passing the force argument.

[plinss]

@plinss is there anyway you could securely pass a stack trace, a formatted error message, or both in the body of the response?

It does return the error response unless you pass the force argument. It's just a dump of whatever bikeshed's stdout and stderr output are. The force argument causes it to only return the generated bikeshed output and never an error message (you just get the http status code and a blank document).

Also note that if you don't use force, content negotiation is in effect, an Accept text/plain will get you the error output, if any, and Accept text/html gets you the generated output.

[plinss]

I also have the 400/500 change ready to go once speced/bikeshed#1699 lands

[tabatkins]

landed

[plinss]

And the api endpoint is updated, you should be getting 400 when bikeshed reports errors, and a 500 if it crashes.

[tobie]

Thanks, folks! Closing.