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

crash during check external_links #1297

Closed
iay opened this Issue Jan 22, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@iay
Contributor

iay commented Jan 22, 2018

bundle exec nanoc check elinks causes exception

Details

kite:site iay$ bundle exec nanoc check elinks
Loading site… done
  Running check external_links…   
Captain! We’ve been hit!

TypeError: no implicit conversion of nil into String

  0. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/bundler/gems/nanoc-6b5b66edc64f/nanoc/lib/nanoc/checking/checks/external_links.rb:84:in `+'
  1. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/bundler/gems/nanoc-6b5b66edc64f/nanoc/lib/nanoc/checking/checks/external_links.rb:84:in `block in validate'
  2. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/bundler/gems/nanoc-6b5b66edc64f/nanoc/lib/nanoc/checking/checks/external_links.rb:62:in `times'
  3. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/bundler/gems/nanoc-6b5b66edc64f/nanoc/lib/nanoc/checking/checks/external_links.rb:62:in `validate'
  4. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/bundler/gems/nanoc-6b5b66edc64f/nanoc/lib/nanoc/checking/checks/external_links.rb:40:in `block in select_invalid'
  5. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/gems/parallel-1.12.1/lib/parallel.rb:484:in `call_with_index'
  6. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/gems/parallel-1.12.1/lib/parallel.rb:342:in `block (2 levels) in work_in_threads'
  7. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/gems/parallel-1.12.1/lib/parallel.rb:495:in `with_instrumentation'
  8. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/gems/parallel-1.12.1/lib/parallel.rb:341:in `block in work_in_threads'
  9. /Volumes/Cased/git/iay/iay/site/vendor/ruby/2.4.0/gems/parallel-1.12.1/lib/parallel.rb:206:in `block (2 levels) in in_threads'

Crash log

https://gist.github.com/iay/2fc13b5b7b406a6b50ef998b7023ef76

@iay

This comment has been minimized.

Show comment
Hide comment
@iay

iay Jan 22, 2018

Contributor

You can see from the traceback that this is the gitlab head from a couple of days ago.

Contributor

iay commented Jan 22, 2018

You can see from the traceback that this is the gitlab head from a couple of days ago.

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Jan 22, 2018

Member

Is this an exception that occurs consistently?

Line 84 of external_links.rb says

location = base_url.to_s + location

That TypeError would occur when location is nil, which can only happen when res['Location'] is nil. That’d be for a HTTP 3xx response without a Location header, which can happen (e.g. for 300 Multiple Choices).

At the very least, Nanoc should check the presence of the Location header, before attempting to do anything with it!

Member

ddfreyne commented Jan 22, 2018

Is this an exception that occurs consistently?

Line 84 of external_links.rb says

location = base_url.to_s + location

That TypeError would occur when location is nil, which can only happen when res['Location'] is nil. That’d be for a HTTP 3xx response without a Location header, which can happen (e.g. for 300 Multiple Choices).

At the very least, Nanoc should check the presence of the Location header, before attempting to do anything with it!

@iay

This comment has been minimized.

Show comment
Hide comment
@iay

iay Jan 22, 2018

Contributor

Yes, it seems to be consistent. I will try and add some kind of puts to nail down the URL in question.

Contributor

iay commented Jan 22, 2018

Yes, it seems to be consistent. I will try and add some kind of puts to nail down the URL in question.

@iay

This comment has been minimized.

Show comment
Hide comment
@iay

iay Jan 23, 2018

Contributor

This seems to be one of the links that is killing things:

kite:site iay$ curl -I https://movabletype.org/default_styles.shtml
HTTP/1.1 302 Moved Temporarily
Content-Type: text/html
Date: Tue, 23 Jan 2018 08:34:27 GMT
Server: nginx
X-Frame-Options: SAMEORIGIN
Connection: keep-alive
Contributor

iay commented Jan 23, 2018

This seems to be one of the links that is killing things:

kite:site iay$ curl -I https://movabletype.org/default_styles.shtml
HTTP/1.1 302 Moved Temporarily
Content-Type: text/html
Date: Tue, 23 Jan 2018 08:34:27 GMT
Server: nginx
X-Frame-Options: SAMEORIGIN
Connection: keep-alive
@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Jan 26, 2018

Member

That HTTP response is weird:

  1. HTTP/1.1 defines 302 as 302 Found, not 302 Moved Temporarily. It seems to mix HTTP/1.0 and HTTP/1.1.

  2. The Location header is supposed to be present.

Not sure what to do in this case. Probably not error, and treat this situation as OK? An error in this case would not provide anything actionable.

Member

ddfreyne commented Jan 26, 2018

That HTTP response is weird:

  1. HTTP/1.1 defines 302 as 302 Found, not 302 Moved Temporarily. It seems to mix HTTP/1.0 and HTTP/1.1.

  2. The Location header is supposed to be present.

Not sure what to do in this case. Probably not error, and treat this situation as OK? An error in this case would not provide anything actionable.

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Jan 26, 2018

Member

#1302 has a fix.

Member

ddfreyne commented Jan 26, 2018

#1302 has a fix.

@ddfreyne ddfreyne closed this in #1302 Jan 26, 2018

ddfreyne added a commit that referenced this issue Jan 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment