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

Errors with service worker js #155

Closed
mohamedhamad opened this issue Jun 2, 2019 · 23 comments · Fixed by #165
Closed

Errors with service worker js #155

mohamedhamad opened this issue Jun 2, 2019 · 23 comments · Fixed by #165
Milestone

Comments

@mohamedhamad
Copy link

mohamedhamad commented Jun 2, 2019

I'm having a hard time getting service workers to be registered and work. Seems like in the chrome console log there is an a js error in the generated service work

?wp_service_worker=1:129 Uncaught SyntaxError: missing ) after argument list

and

(index):1 Uncaught (in promise) TypeError: Failed to register a ServiceWorker: ServiceWorker script evaluation failed

and when using Lighthouse extension in chrome, I get the following errors

  • Current page does not respond with a 200 when offline
  • start_url does not respond with a 200 when offline. Unable to fetch start URL via service worker.
  • Does not register a service worker that controls page and start_url

on my local dev environment, I get non of these issues and its a very similar install. the one thing that does happen in my local dev environment is if the plugin is installed and I add the theme support, the page continually refreshes as the service works are working and updating non-stop.

I'm not really using many of the functions as I'm experimenting.

add_theme_support( 'service_worker', true );

/**
 * WEB APP Manifest Filters
 *
 * @param array $manifest
 * @return array $manifest
 */
add_filter('web_app_manifest', 'wundertheme_web_app_manifest_filter', 1, 1);
function wundertheme_web_app_manifest_filter($manifest)
{
    $manifest['theme_color'] = '#D9422B';
    $manifest['short_name']  = 'Mo Hamad';
    $manifest['orientation'] = "any";
    $manifest['display']     = "standalone";
    return $manifest;
}

/**
 * Adds an HSTS header to the response.
 *
 * @param array $headers The headers to filter.
 * @return array $headers The filtered headers.
 */
add_filter( 'wp_headers', function( $headers ) {
    $headers['Strict-Transport-Security'] = 'max-age=3600'; // Or another max-age.
    return $headers;
});

wp_register_service_worker_caching_route(
    '/.*\.(?:png|gif|jpg|jpeg|svg|webp|css|js)(\?.*)?$',
        array(
            'strategy'  => WP_Service_Worker_Caching_Routes::STRATEGY_CACHE_FIRST,
            'cacheName' => 'assets',
            'plugins'   => array(
                'expiration'        => array(
                    'maxEntries'    => 60,
                    'maxAgeSeconds' => 60 * 60 * 24,
            ),
        ),
    )
);

if anyone is so inclined, you can check out the lighthouse results for the site I'm working on
https://www.mohamed-hamad.com/

I'm also testing with https://www.pwabuilder.com, and their tool picks up everything but the service worker.

is there a tutorial on how to get the basics in, or an article that goes through the options and how to set it up?

@westonruter
Copy link
Collaborator

@mohamedhamad thanks a lot for sharing the URL for your site. There is indeed a JS syntax error in the service worker which is preventing it from installing.

This is the service worker JS response: https://www.mohamed-hamad.com/?wp_service_worker=1

I'll look into what is going on.

Have you registered any custom service worker logic?

@mohamedhamad
Copy link
Author

hey @westonruter, thanks for having a look at things.
for now, keeping it simple, and not adding any custom service worker logic.

@westonruter
Copy link
Collaborator

For reference, this is the syntax error being emitted from the document:

image

This is the line in question:

https://github.com/xwp/pwa-wp/blob/a5894cf850b8ce94ac441db775c6835ceed9c8f0/wp-includes/js/service-worker-offline-commenting.js#L32

@mohamedhamad do you by chance have any optimizer plugin running which strips out all HTML comments from PHP responses?

@westonruter
Copy link
Collaborator

westonruter commented Jun 3, 2019

That appears to be the case. If so, try a patch like this (or I can open a PR) which masks the presence of the HTML comments:

diff --git a/wp-includes/js/service-worker-navigation-routing.js b/wp-includes/js/service-worker-navigation-routing.js
index 3231a0d..a0c7be3 100644
--- a/wp-includes/js/service-worker-navigation-routing.js
+++ b/wp-includes/js/service-worker-navigation-routing.js
@@ -65,9 +65,9 @@ ERROR_OFFLINE_BODY_FRAGMENT_URL, STREAM_HEADER_FRAGMENT_QUERY_VAR, NAVIGATION_BL
 							headers: errorResponse.headers
 						};
 
-						let body = text.replace( /<!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.error );
+						let body = text.replace( /[<]!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.error );
 						body = body.replace(
-							/(<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->)((?:.|\n)+?)(<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->)/,
+							/([<]!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->)((?:.|\n)+?)([<]!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->)/,
 							( details ) => {
 								if ( ! responseBody ) {
 									return ''; // Remove the details from the document entirely.
@@ -86,8 +86,8 @@ ERROR_OFFLINE_BODY_FRAGMENT_URL, STREAM_HEADER_FRAGMENT_QUERY_VAR, NAVIGATION_BL
 								details = details.replace( '{{{iframe_srcdoc}}}', srcdoc );
 
 								// Replace the comments.
-								details = details.replace( '<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->', '' );
-								details = details.replace( '<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->', '' );
+								details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->', '' );
+								details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->', '' );
 								return details;
 							}
 						);
@@ -111,7 +111,7 @@ ERROR_OFFLINE_BODY_FRAGMENT_URL, STREAM_HEADER_FRAGMENT_QUERY_VAR, NAVIGATION_BL
 						headers: response.headers
 					};
 
-					const body = text.replace( /<!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.default );
+					const body = text.replace( /[<]!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.default );
 
 					return new Response( body, init );
 				} );
diff --git a/wp-includes/js/service-worker-offline-commenting.js b/wp-includes/js/service-worker-offline-commenting.js
index 59bea59..6d4fbe9 100644
--- a/wp-includes/js/service-worker-offline-commenting.js
+++ b/wp-includes/js/service-worker-offline-commenting.js
@@ -29,9 +29,9 @@
 								headers: errorResponse.headers
 							};
 
-							let body = text.replace( /<!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.error );
+							let body = text.replace( /[<]!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.error );
 							body = body.replace(
-								/(<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->)((?:.|\n)+?)(<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->)/,
+								/([<]!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->)((?:.|\n)+?)([<]!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->)/,
 								( details ) => {
 									if ( ! errorText ) {
 										return ''; // Remove the details from the document entirely.
@@ -50,8 +50,8 @@
 									details = details.replace( '{{{iframe_srcdoc}}}', srcdoc );
 
 									// Replace the comments.
-									details = details.replace( '<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->', '' );
-									details = details.replace( '<!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->', '' );
+									details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN--' + '>', '' );
+									details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END--' + '>', '' );
 									return details;
 								}
 							);
@@ -93,7 +93,7 @@
 							headers: response.headers
 						};
 
-						const body = text.replace( /<!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.comment );
+						const body = text.replace( /[<]!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.comment );
 
 						return new Response( body, init );
 					} );

@mohamedhamad
Copy link
Author

@westonruter yes I do have a plugin to optimize the site and does strip out html comments.
I can try and remove to see if helps.

@mohamedhamad
Copy link
Author

ok, so disabling the html comment stripping in the Asset Cleanup plugin seems to have worked. I cleared the cache and tested in the chrome lighthouse audit extension and I'm getting all checks validated.
I am however getting the following error in the console relating to IDBDatabase and workbox

workbox-core.prod.js:1 Uncaught (in promise) DOMException: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
    at https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-core.prod.js:1:1599
    at new Promise (<anonymous>)
    at r.transaction (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-core.prod.js:1:1564)
    at async r.u (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-core.prod.js:1:1718)
    at async r.put (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-core.prod.js:1:2151)
    at async o.setTimestamp (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-expiration.prod.js:1:562)
    at async updateTimestamp (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-expiration.prod.js:1:1532)
    at async Object.cacheDidUpdate (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-expiration.prod.js:1:2563)
    at async Object.put (https://www.mohamed-hamad.com/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-core.prod.js:1:4062)

@westonruter
Copy link
Collaborator

@mohamedhamad I've noticed that sometimes after clearing caches. If you close DevTools and reload and re-open, that should go away.

@westonruter
Copy link
Collaborator

westonruter commented Jun 3, 2019

The Asset Cleanup plugin should really be checking the Content-Type of the response before it just blindly strips all HTML comments.

@mohamedhamad
Copy link
Author

amazing! thank you @westonruter
by any chhance, is there any useful tutorials and guides on how to setup this plugin in the real world. I'm just playing around with it and want to get to know it well before I start rolling it out in everything I do. this is site is a playground for me.

@westonruter
Copy link
Collaborator

@mohamedhamad One thing is @amedina and I gave a talk in part about this plugin at CDS last year: https://youtu.be/s1WrBaAyzAI

Otherwise, there is the API reference in the readme.

See also this comment: #113 (comment)

The plugin is really looking for the overall patterns to emerge for how to leverage across the various WordPress use cases, so it's hard to pin down any extensive tutorial about how to use.

One important resource to get familiar with is Workbox.js, since the plugin here is in big part a wrapper around the functionality it provides. There are some great tutorials there which give you a feel for how to build logic for the service worker which you should be able to map easily into the PWA plugin (e.g. using the PHP API).

@mohamedhamad
Copy link
Author

awesome, thanks for the pointers @westonruter
will check out the video and dig in to the documentation

@westonruter
Copy link
Collaborator

@mohamedhamad Would you please re-enable the HTML comment stripping and test #165 to see if it works around the problem?

@mohamedhamad
Copy link
Author

just tried it out, the updated file is still up there if you wanna have a look. similar issue as before

@westonruter
Copy link
Collaborator

@mohamedhamad I'm not seeing any JS errors like before. Seems to be working.

@mohamedhamad
Copy link
Author

@westonruter I’ve been toggling the HTML comment stripping on n off to test on different browsers. It’s definitely the same issue. Will turn it on again n leave it for a while for you to see

@westonruter
Copy link
Collaborator

There is no JS error for me. And when looking at https://www.mohamed-hamad.com/?wp_service_worker=1 there is no syntax error. Likewise, I can see the expected code:

						let body = text.replace( /[<]!--WP_SERVICE_WORKER_ERROR_MESSAGE-->/, errorMessages.error );
						body = body.replace(
							/([<]!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->)((?:.|\n)+?)([<]!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->)/,
							( details ) => {
								if ( ! responseBody ) {
									return ''; // Remove the details from the document entirely.
								}
								const src = 'data:text/html;base64,' + btoa( responseBody ); // The errorText encoded as a text/html data URL.
								const srcdoc = responseBody
									.replace( /&/g, '&amp;' )
									.replace( /'/g, '&#39;' )
									.replace( /"/g, '&quot;' )
									.replace( /</g, '&lt;' )
									.replace( />/g, '&gt;' );
								const iframe = `<iframe style="width:100%" src="${src}" data-srcdoc="${srcdoc}"></iframe>`;
								details = details.replace( '{{{error_details_iframe}}}', iframe );
								// The following are in case the user wants to include the <iframe> in the template.
								details = details.replace( '{{{iframe_src}}}', src );
								details = details.replace( '{{{iframe_srcdoc}}}', srcdoc );

								// Replace the comments.
								details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->', '' );
								details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->', '' );
								return details;
							}
						);

						return new Response( body, init );

@mohamedhamad
Copy link
Author

I definitely see it still. I cleared my cache. restarted the browser. on first load its fine. but when I navigate around, I see a console error and an error in the application tab of chrome developer tools

@westonruter
Copy link
Collaborator

Strange. I just opened your site in an incognito window and now I see it too.

Are you sure you deployed the changes from the PR? I'm not seeing expected code.

In particular, I'm seeing:

// Replace the comments.
details = details.replace( '', '' );
details = details.replace( '', '' );

But in my PR, I changed the strings around to use concatenation and so I'd expect to see + or something in the output.

@mohamedhamad
Copy link
Author

Yea, I pulled the file and uploaded directly to the server.
These are some of the main changes I see.. just copied this from the server

// Replace the comments.
details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_BEGIN-->', '' );
details = details.replace( '<' + '!--WP_SERVICE_WORKER_ERROR_TEMPLATE_END-->', '' );
return details;

@westonruter
Copy link
Collaborator

@mohamedhamad It looks like you did replace the one file, but there is another file that also had changes. Please update both service-worker-navigation-routing.js and service-worker-offline-commenting.js.

@mohamedhamad
Copy link
Author

oops, apologies, my bad. updated the second file and it looks good.
you can have a look. thanks for working it out with me.

@westonruter
Copy link
Collaborator

@mohamedhamad great, I'm not seeing any JS errors. If you don't either, I'll go ahead and merge the PR.

@mohamedhamad
Copy link
Author

did a site wide test, no errors. I think its good to go

@westonruter westonruter added this to the 0.3 milestone Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants