-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix Hyperlink/Travis Issues #1582
Conversation
4a687e1
to
eab49af
Compare
@Munter I figured out the issue here and it's actually more a problem with our customizations to the The issue is that we don't output anything until I'm not too familiar with the |
I think the solution might be to use an appropriate tap-reporter or write your own with a filter. Most tap reporters I have seen have continuous reporting of the tap stream. Alternatively you could use tee to pipe the tap output to a file while still logging all the activity to the console, then post-process the tap output from the file |
Yeah so I updated our script to check and output line by line, instead of waiting for completion, but unfortunately as you can see the build still fails for some reason. There must be one link check that's hanging for over 10 min. I'm thinking a quick hack would just be to use a |
@Munter somehow it's still hanging on the link following this line:
(this is with the patch fix I made for your |
@Munter btw I'd be more than happy to add you as a contributor if you're interested. It might be easier for you to debug this branch/pr/ci-build that way. |
@skipjack yeah, that might be useful. I'm not sure when I'll have the time to do a deep dive on this, but when I do it should speed up my feedback loop |
No worries, just sent you the collaborator invite. For now I'm just running |
@pierreneter would you be interested in submitting a PR to the I've been pushing hard to get the site's information architecture sorted out and sections cleaned up which I think will be pretty complete by the end of this week. I have a plan on how to start working at #1525 and #1400 starting next week (12/24-ish), assuming all goes to plan. Hoping to review that with you and @jeremenichelli if you're both still interested and willing to help. That said, it would be good to get this resolved before we dig into that though as |
Ok. I will try |
@skipjack I'm in for the next steps towards this. Just ping me on the right discussions, I haven't been in Slack too much the last weeks because of personal stuff. I think that moving to another static generator like jekyll, hugo, gatsby, etc. is worth a hangout to see what's best given the complexity of a hypothetical migration and the amount of work that it will mean. I personally would love that to happen :) |
About #1525, #1400. The same with @jeremenichelli, I vote for |
See logs at https://travis-ci.org/webpack/webpack.js.org/builds/318201104 It looks like set |
It's stuck at a font file ( |
@pierreneter I'm actually not sure it's hanging on the font file even though it's the last log line: loading build/314bbcd238d458622bbf32427346774f.woff I think it may be loading the next one but failing before it makes to logging it out (though I could be wrong). I'm pretty sure it's one of the sponsor/backer links. I think we have to debug more at the |
@skipjack @Munter added abort a request after 30 seconds timeout (Munter/hyperlink@a0bb4b3) but I don't know it works or not. |
@jeremenichelli Cool, no problem. Yeah I was thinking maybe you could take the lead on the refactoring the
@pierreneter @jeremenichelli yeah if we're going to switch to another SSG, I think Gatsby would be the best alternative. @pierreneter I did finally track down the How about this, I'll throw together a branch within the next two weeks to demo what I mean on the pure webpack approach. If that feels clunky, we can take a stab at porting to Gatsby. If not, maybe we continue down the pure webpack path and see where it takes us. Either of those approaches would leave us in a better spot than we are now and would make the tasks in #1525 and #1400 easier to tackle. The question in my mind is more, do we port to Gatsby and let it handle everything for us behind the scenes or can we build a relatively simple webpack config that yields the same thing but leaves us with more control if needed (btw, Gatsby utilizes the Let's not clutter this thread more (my bad 😁 ) but I really will ping you guys soon to finish discussing infrastructure. |
@pierreneter could you use a git dependency in the |
@pierreneter actually I think that commit was the one from a while ago that we tested out but didn't work. Feel free to try again though. |
I see several possible causes of error here. Might be some request in a promise that throws unexpectedly and is not caught. Or some crazy long download or an actual timeout. I was considering pushing everything that initiates a request in some sort of request tracking array and then removing it again when the request has resolved. Then start logging out that arrays content periodically , which means if there is a 10 minute hang that stops the CI server, we should see the unhandled connections logged in the console. That would let us know what resources cause this and possibly how to recreate it locally to implement a test against it |
I added more logs to |
@Munter any idea why the I was initially thinking we could do the same thing with a
Where can we get access the requests? Somewhere from within the |
Since hyperlink is set up in a way right now where it will fail the build, I think we need to do a few tweaks to reduce either the scope of the tests or the severity. This version of hyperlink has a I see a few different ways of going about this: Mark test of imported content as
|
The CI tests are passing on this branch now, but I don't think we are ready to merge just yet. See #1582 (comment) |
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.
Hyperlink tap-spot needs to be updated to fail the build on error.
There needs to be a good way to separate critical errors from warnings in hyperlink and still being able to follow up on them even if they don't break the build
Hyperlink should be updated to the upcoming v4 release when it is out
@Munter Sounds good |
The change set was getting a bit unwieldy, so I squashed everything to keep this branch manageable. The change set is quite small after all the experiments. No need to keep that history |
…pot reporter fix(tests): fix build termination issues with `lint:links` refactor(tests): test out `defaultTimeout` branch refactor(tests): test out syntax error patch for hyperlink fix Update hyperlink version Use --verbose option to determine the problem Try my custom hyperlink add limit assets loading per time Change limit to 60 Remove concurrency of hyperlink Switch hyperlink to assetgraph4 feature branch The upcoming major version of Hyperlink is being built here, and we're seeing good improvements on both memory consumption and runtime Switch to node 8 on Travis Hyperlink uses Assetgraph 4, which has node 8 as a minimum version. Add extremely verbose memory output to hyperlink Update hyperlink assetgraph dependency to 4.0.5 This clears out memory leaks from javascript assets where jsdom documents were not properly closed Update hyperlink Switch to raw hyperlink tap output to see more context Update hyperlink reference to include assetgraph 4.1.1 Bring back check-links tap filtering Updated hyperlink and use new skipfilters and tap reporter Update hyperlink to version that doesn't fail on svg redirects and gives false negative content-type-mismatch errors Remove check-links.js. hyperlink --skip and tap-spot do the same Removed tap-parser dependency, which was only used in now deleted check-links script
…aw.githubusercontent.com
@Munter What else is missing to merge this one? |
@montogeek I can't build the current master on my machine, so I can't iterate. I need the updated rebuild branch merged before I can continue |
@Munter Why it is not building for you? :/ |
I can't wait 50 minutes for a build, which is what the current master takes on my MacBook air |
@Munter Would it help if I send you a zip file with |
Not really. I need the ability to iterate and fix things as I go |
@Munter if that's the case, then maybe rebase on |
I'm still strapped on time but would love to see this finished. If anyone has bandwidth to dig into it before I do -- which might still be a few months out -- I highly recommend rebasing on |
I could do it tomorrow |
CLosing in favor of #2382 |
* Update link checking to use latest hyperlink. Replaces #1582 * Stop appending empty fragment to urls pointing ad no fragment identifier * Resolve relative hrefs from README's to their github.com url instead of linking locally inside docs * Prefer https where available and also use https in examples * Make non-fragment links inside headings visible * Update webpack docs china link * Fixed invalid edit links to index pages * skip fragment check for vimdoc that is known to fail * Fix broken link to migration guide from configuration/resolve * Also resolve github README relative links when they don't start with a dot * Add missing pages: Vote, Organization, Starter kits * Remove unused code * Fix link that was redirected * Open all target='_blank' links with rel='noopener' for security * Skip link checking img.shields.io * Change order of markdown url rewriting to avoid errors * Use lowercase variable name for node url module. See #2382 (comment) * Update link to file-loader github repo * Rewrite links from https://npmjs.com to https://www.npmjs.com to avoid the inevitable redirect * Skip checking links to npmjs.com/package that causes hyperlink to get http 500 responses. This should probably be reverted and fixed later * Updated organization projects urls to their redirect target * Update tool-list to get correct links for projects * Updated links to their redirect target * Remove jscs-loader. The project is deprecated, merged with eslint and the domain we link to is for sale * Update link to karma-webpack project * Remove link to named-modules-plugin, which is now in webpack core * Added missing mini-css-extract-plugin to fetch script * Skip checking links to known deleted github users * Update broken fragments to work with new markdown generator * Fixed more broken links * infra(site) Copy assets when building * Update github location for css-loader * Remove dead github accounts from contributors listings * infra(site) Fix fetchPackages script when running locally * Fix internal fragmtn links in optimization.md * Skip link checking or opencollective.com/webpack. Massive html response made the checker go into CPU overdrive * Remove link to non-existing named-modules-plugin * Use new release of hyperlink * feat(infra) Travis optimization (#2404) * Fix internal fragmtn links in optimization.md * Skip link checking or opencollective.com/webpack. Massive html response made the checker go into CPU overdrive * Try out travis staged build * Add proselint * Upgrade pip before using it * Move before-hooks around to try and make proselint install * Try adding python as a language as well to geet an updated version * More messing with config * Manually handle node versioning * Add node minor version to nvm install. Defaulted to slightly too low version * Manually install node 8.11 * Try a matrix build to separate node and python stuff * Add linkcheck task and try a different cahce setup * Minor name change to test if cache works correctly across builds * Attempt to combine matrix and staged build * Attempt going back to staged build * Bump travis * Ping Travis. You alive? * Fix broken travis.yml * Fix wrong stage order * Explicitly run specific lintings, exclude proselint * Allow failures for link checker * Change proselint cache directory. Maybe this works * Add new script to fetch repository names that the docs should contain. Try to centralize github API usage to avoid rate limitations * Add caching to eslint * Remove parts of deploy.sh that are now run by travis * Added new ./repositories to store github api results * Replace fetch.sh with npm scripts and fetch-package-readmes.js * Attempt to make caches more specific to the containers and stages they refer to * Remove deprecaed fetch_packages script. Now replaced by two-step fetch-package-repos and fetch-package-readmes * Attempt a more windows friendly build by using npm and webpack instead of shell commands * Disable link checking for now to speed up builds * Revert "Disable link checking for now to speed up builds" This reverts commit 7d4bb06. * Add dist to proselint cache so generated content also gets checked * Remove unnessessary GITHUB_TOKEN env variable check in repo fetching script * Revert "Add dist to proselint cache so generated content also gets checked" This reverts commit fc7676d. * Rework pipeline for better speed and make proselint a deployment blocker * Rename build stage to reflect its actions * Run content-tree only after generating all external content * Remove link to non-existing named-modules-plugin * Fix double slashes in edit links * Rename stages * Add new internal link check as a blocking check * Fix wrong name in allowed_failures config
Reverted 8281949 so we can test and try to resolve the
hyperlink
issue on Travis' network. I think there still may be one link failure due to a package's README that we may have to resolve before we can fix the core issue.Related to Munter/hyperlink#121