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.

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

sitemap.xml throws error #20

Closed
jaro-io opened this issue Jan 26, 2023 · 10 comments
Closed

sitemap.xml throws error #20

jaro-io opened this issue Jan 26, 2023 · 10 comments
Labels
needs: discussion 🗣 Requires further discussion to proceed

Comments

@jaro-io
Copy link

jaro-io commented Jan 26, 2023

hey again 🐰
since i installed the plugin my sitemap is not available anymore. any ideas what’s happening here?

Screenshot 2023-01-26 at 23 31 47

@jaro-io jaro-io changed the title sitemap throws error sitemap.y throws error Jan 26, 2023
@jaro-io jaro-io changed the title sitemap.y throws error sitemap.xml throws error Jan 26, 2023
@lukasbestle
Copy link
Member

Do you use a plugin to generate the sitemap? If so, which one?

@lukasbestle lukasbestle added the missing: information ❓ Requires more information to proceed label Jan 27, 2023
@jaro-io
Copy link
Author

jaro-io commented Jan 27, 2023

@lukasbestle ah, yes indeed.

fabianmichael/kirby-meta

@lukasbestle
Copy link
Member

Thanks for the link. The plugin uses the Kirby page cache, but with a custom data format that is incompatible with page responses that are cached by Kirby:

https://github.com/fabianmichael/kirby-meta/blob/23f2f8fd80991b2ff132c4c54b3f138d5957d1a0/config/routes.php#L41-L50

Kirby stores an array with a html key (= body string) and a response key with response metadata.

The compatibility issue can be solved in either the kirby-meta plugin or in Staticache. However the most robust solution would be one in kirby-meta as it will also be compatible with the core page cache and other caching plugins.

Pinging @fabianmichael.

@lukasbestle lukasbestle added needs: discussion 🗣 Requires further discussion to proceed and removed missing: information ❓ Requires more information to proceed labels Jan 29, 2023
@fabianmichael
Copy link

@lukasbestle I’d be happy to update my plugin, but hit a roadblock. The StatiCache class assumes that the response to generate the file from is a regular page, which can be found by calling $kirby->page(), which is not true for the sitemap. So far it has always been a simple response, but I just tried to use a virtual page instead, to provide the plugin the regular page API. But unfortunately, that means that sitemap.xml is still not considered a child page of $site and thus cannot be found by the page() method. Any idea or does staticache needs to be updated as well?

https://github.com/getkirby/staticache/blob/main/src/StatiCache.php#L106-L107

@lukasbestle
Copy link
Member

@fabianmichael You are right, this is something that can only be fixed in Staticache. What we could do is to automatically fall back to Url::to($key['id']) if the page was not found. Not a full fallback because of the language code that won't be respected in this case, but at least for the sitemap use case it should be enough.

@fabianmichael
Copy link

@lukasbestle Sounds like a start. I don’t understand the whole things that goes on internally when Kirby renders, caches and serves a page. But as far as I understand, this would do the trick. As soon as staticache is updated, I will update my plugin.

@jonasfeige
Copy link

Any ETA on this? Would be great if both plugins could be used together.

@lukasbestle
Copy link
Member

lukasbestle commented Feb 23, 2023

@jonasfeige Sorry for the delay. I can't give you an ETA for an official fix, but you could temporarily replace this line with the following:

-		$url  = $page->url($key['language']);
+		$url  = $page?->url($key['language']) ?? Url::to($key['id']);

@bnomei
Copy link

bnomei commented Mar 8, 2023

@fabianmichael i just added a possible solution here: fabianmichael/kirby-meta#39

@fabianmichael
Copy link

I’ve updated my plugin to always use its own cache. That way, the sitemap cannot be served static, but the CMS does not crash any longer (see dd6bdecc5c65237b11f505ddc37179099b0f4a15). As soon as someone confirms me that it works on their setup, I’ll create a new release of the meta plugin.

@lukasbestle lukasbestle mentioned this issue Mar 11, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: discussion 🗣 Requires further discussion to proceed
Projects
None yet
Development

No branches or pull requests

5 participants