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

wp-now crashes when offline / DNS lookup cannot resolve #535

Closed
GaryJones opened this issue Jun 8, 2023 · 6 comments · Fixed by WordPress/playground-tools#226
Closed

wp-now crashes when offline / DNS lookup cannot resolve #535

GaryJones opened this issue Jun 8, 2023 · 6 comments · Fixed by WordPress/playground-tools#226
Labels
[Feature] PHP.wasm [Type] Bug An existing feature does not function as intended
Milestone
PHP Feature Parity

Comments

@GaryJones
Copy link
Member

When working offline (such as on an airplane), trying to navigate into the WP admin causes wp-now to crash:

Screenshot 2023-06-07 at 08 31 38

Steps to reproduce

  1. Run wp-now start in a plugin or theme directory or other startup method.
  2. Disable internet connection at the device level.
  3. Use DevTools Network tab to Disable Cache and throttle to be Offline:
Screenshot 2023-06-08 at 14 25 42
  1. Navigate to wp-admin on the opened site e.g. http://127.0.0.1:8881/wp-admin/
  2. Note that the page fails to load, and the terminal shows the error message (per screenshot):
...
Server running at http://127.0.0.1:8881/
/Users/xxx/.nvm/versions/node/v18.10.0/lib/node_modules/@wp-now/wp-now/node_modules/@php-wasm/node/index.cjs:17817
      throw reason;
      ^

Error: getaddrinfo ENOTFOUND api.wordpress.org
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:107:26) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'api.wordpress.org'
}

Node.js v18.10.0
@danielbachhuber danielbachhuber added [Type] Bug An existing feature does not function as intended [Feature] PHP.wasm labels Jun 8, 2023
@danielbachhuber danielbachhuber mentioned this issue Jun 8, 2023
Closed
@danielbachhuber danielbachhuber mentioned this issue Aug 3, 2023
Open
@adamziel adamziel mentioned this issue Nov 22, 2023
Open
@adamziel
Copy link
Collaborator

adamziel commented Dec 21, 2023 •
edited
Loading

CC @sejas , although the stack trace seems to be pointing to @php-wasm/node.

@adamziel adamziel added this to the PHP Feature Parity milestone Feb 29, 2024
@fluiddot fluiddot mentioned this issue Apr 9, 2024
Merged
@fluiddot
Copy link
Contributor

fluiddot commented Apr 9, 2024

👋 I created this PR that aims to address this issue: #1215. Let me know if you have any thoughts/feedback, thanks 🙇 !

adamziel pushed a commit that referenced this issue Apr 9, 2024
@fluiddot
Catch DNS errors to avoid unhandled exceptions (#1215)
c340b0d 
Catches potential DNS errors produced in outbound network requests to
avoid unhandled exceptions.

## What problem is it solving?
Fixes the problem described in
#535.

## How is the problem addressed?
The PR introduces a `try-catch` statement to catch errors produced when
invoking `lookup` to resolve the target host address. When this happens,
it sends empty binary data as a notification that the connection
initiated but soon after, it closes the connection with error code
`3000`.

## Testing Instructions
The [original
issue](#535)
references `wp-now`, so it's recommended to test the changes using a
local build of `wp-now` from the [`playground-tools`
repository](https://github.com/WordPress/playground-tools).

1. In `wordpress-playground` repository, run `nx build php-wasm-node`.
2. Check out [`playground-tools`
repository](https://github.com/WordPress/playground-tools).
3. In `playground-rools` repository, update the `@php-wasm/node`
dependency to point to the local path of the `wordpress-playground`
repository.

**Example:**
```patch
diff --git forkSrcPrefix/package.json forkDstPrefix/package.json
index cc623dc57810509691383fa068cc8f921c957f48..ac84b7f467c2f7d23b9fa8c5f9487cafdb4644e6 100644
--- forkSrcPrefix/package.json
+++ forkDstPrefix/package.json
@@ -32,7 +32,7 @@
 		"@codemirror/state": "6.2.0",
 		"@codemirror/theme-one-dark": "6.1.1",
 		"@codemirror/view": "6.9.3",
-		"@php-wasm/node": "0.6.10",
+		"@php-wasm/node": "file:../wordpress-playground/dist/packages/php-wasm/node",
 		"@php-wasm/progress": "0.6.10",
 		"@php-wasm/universal": "0.6.10",
 		"@php-wasm/web": "0.6.10",

```
4. Run `npm i`.
5. Run `nx build wp-now`.
6. Run `node dist/packages/wp-now/cli.js start`.
7. Turn off internet connection.
8. Navigate to the site's WP admin (`/wp-admin`).
9. Observe no errors are logged and that `wp-now` doesn't crash.
10. Navigate to `/wp-admin/plugin-install.php`.
11. Observe no errors are logged and that `wp-now` doesn't crash.
12. Observe that the page shows an error message due to being offline.
13. Navigate to `/wp-admin/update-core.php`
14. Observe no errors are logged and that `wp-now` doesn't crash.

> [!NOTE]
> It's recommended to disable the network cache in your browser via the
DevTools Network tab.
@adamziel
Copy link
Collaborator

adamziel commented Apr 9, 2024 •
edited
Loading

I just merged the fix, thank you @fluiddot! Let's close this issue once wp-now releases a new version.

@sejas
Copy link
Collaborator

sejas commented Apr 9, 2024

@adamziel , I cannot see the new version on npm.
https://www.npmjs.com/package/@php-wasm/node

Expected: 0.6.12 , Current: 0.6.10
Do we need to take any additional step?

@fluiddot
Copy link
Contributor

fluiddot commented Apr 9, 2024

Not sure if it's related but I noticed recent Release NPM packages jobs failed.

@sejas sejas mentioned this issue Apr 10, 2024
Merged
@sejas
Copy link
Collaborator

sejas commented Apr 10, 2024

This is fixed on wp-now 0.1.69 https://www.npmjs.com/package/@wp-now/wp-now/v/0.1.69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Feature] PHP.wasm [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
PHP Feature Parity
Development

Successfully merging a pull request may close this issue.

Update Playground dependencies to v0.6.13 WordPress/playground-tools
5 participants
@danielbachhuber @GaryJones @adamziel @sejas @fluiddot