Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Issue with sitemap: null #451

Closed
eDubrovsky opened this issue May 17, 2021 · 20 comments · May be fixed by mansong1/golangci-lint#3, irinazheltisheva/golangci-lint#3 or geraldywy/golangci-lint#4
Closed

Issue with sitemap: null #451

eDubrovsky opened this issue May 17, 2021 · 20 comments · May be fixed by mansong1/golangci-lint#3, irinazheltisheva/golangci-lint#3 or geraldywy/golangci-lint#4
Labels
released

Comments

@eDubrovsky
Copy link

Hi. After the plugin update, I'm getting an error when building my Gatsby page. In the configuration, I use sitemap: null

Error:


"gatsby-plugin-robots-txt" threw an error while running the onPostBuild lifecycle:

Invalid URL: null

  73 |   ) {
  74 |
> 75 |     mergedOptions.sitemap = new URL(path.posix.join(pathPrefix, 'sitemap', 'sitemap-index.xml'), mergedOptions.host).toString();
     |                             ^
  76 |   } else {
  77 |     try {
  78 |       new URL(mergedOptions.sitemap)

File: node_modules/gatsby-plugin-robots-txt/src/gatsby-node.js:75:29



  Error: TypeError [ERR_INVALID_URL]: Invalid URL: null
@eDubrovsky eDubrovsky changed the title Issue with sitemap: nul Issue with sitemap: null May 17, 2021
@holm
Copy link

holm commented May 18, 2021

I am seeing the same issue after the update. The plugin documentation explicitly recommends setting sitemap to null for dev/test.

@holm
Copy link

holm commented May 18, 2021

Seems to have come from the changes in #441

@laurenskling
Copy link

yes, #441 breaks the possibility to have sitemap: null as stated in the documentation

@holm
Copy link

holm commented May 21, 2021

cc @moonmeister

@moonmeister
Copy link
Contributor

Yup, we just need a null check and some tests probably. I'll do it if I find time.

@laurenskling laurenskling mentioned this issue May 27, 2021
Closed
@mdreizin
Copy link
Owner

@moonmeister @laurenskling @eDubrovsky I am really sorry, but I don't have a free time to fix that issue. I would appreciate if someone could look at it and fix.

@moonmeister
Copy link
Contributor

moonmeister commented Jun 2, 2021 •
edited
Loading

@mdreizin or whomever...I'm trying to understand how you're getting an error thrown here. the lines 59-69 of this code should check for this null value and properly fill in the site url from the siteUrl metadata. So if this value is truly null then you must not be setting siteUrl. The whole point of setting the plugin config sitemap value to null is to fallback to that other value...if both are null we can provide a cleaner error but there's nothing we can do. Can you better explain what's happening or what you expect to happen? Links to your gatsby-config.js files could be helpful in fixing this bug. Thanks

@moonmeister moonmeister mentioned this issue Jun 3, 2021
Merged
@moonmeister
Copy link
Contributor

Just realized those existing if statements aren't null checks, they're checking for existing properties, null or not. I'll look at this again and see what what I can figure out.

angelxehg pushed a commit to angelxehg/angelxehg.github.io that referenced this issue Jun 15, 2021
disable robots.txt while mdreizin/gatsby-plugin-robots-txt#451 is solved
66ee162
lloydh added a commit to lloydh/gatsby-plugin-robots-txt that referenced this issue Jun 21, 2021
@lloydh
Fixes mdreizin#451
67c8922 
Restores ability to set `sitemap` or `host` to `null` as documented in project README.

Existing tests passing but new tests will be needed to prevent similar bugs in future.
@wezter96
Copy link

wezter96 commented Jul 2, 2021

Has this issue been resolved?
I seem to get the same issue.

@moonmeister
Copy link
Contributor

I don't believe so.

@wezter96
Copy link

wezter96 commented Jul 2, 2021 •
edited
Loading

@moonmeister could I just reroll to an older version of the plugin and then this issue wouldn't exist?
or should i change sitemap: null to something else?

@moonmeister
Copy link
Contributor

Sure but you'll have other issues that this update solved. I may have time to check this out soon. Feel free to contribute a fix too.

lewismorris pushed a commit to lewismorris/gatsby-plugin-robots-txt that referenced this issue Jul 7, 2021
Fixes mdreizin#451 Add null checks to both host and sitemap propertie…
c3550a6 
…s. Update tests.
@lewismorris lewismorris mentioned this issue Jul 7, 2021
Merged
@laurenskling
Copy link

Can we please have a release of the fix? 😅

@mdreizin
Copy link
Owner

mdreizin commented Aug 6, 2021

Please check the latest version.

@Dananz
Copy link

Dananz commented Aug 7, 2021

Please check the latest version.

@mdreizin I don't think that the npm version is synced with the current repo version.

@laurenskling
Copy link

The merge isn't released into a release, would that be possible @mdreizin ?

@eDubrovsky
Copy link
Author

@mdreizin this fix not merged into the 1.6.8 version. 😿

@CassieSloan
Copy link

Updated to most recent version of gatsby-plugin-robots-txt and our builds aren't running bc of the above. Any updates on when this will be fixed?

@iamskok
Copy link

iamskok commented Aug 15, 2021

@mdreizin Please re-open the issue. The fix wasn't merged in 1.6.8 CI output

semantic-release didn't generate a new version. I think it happened because PR didn't include a commit message with the word fix.

@github-actions
Copy link

🎉 This issue has been resolved in version 1.6.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@snyk-bot snyk-bot mentioned this issue Sep 21, 2021
Open
@snyk-bot snyk-bot mentioned this issue Dec 2, 2021
Open
@snyk-bot snyk-bot mentioned this issue Jan 30, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
9 participants
@mdreizin @holm @moonmeister @iamskok @laurenskling @wezter96 @Dananz @eDubrovsky @CassieSloan