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

fix: NodeJS 18 compat #275

Merged
merged 4 commits into from Mar 2, 2023
Merged

fix: NodeJS 18 compat #275

merged 4 commits into from Mar 2, 2023

Conversation

cplepage
Copy link
Contributor

@cplepage cplepage commented Dec 4, 2022

The iOS deployment was silently failing with NodeJS 18.

The src/ios/lib/protocol/protocol.ts:sendMessage(msg, callback) was simply not resolving due to an error while writing on the socket.

After adding

this.socket.on('error', err => {throw err});

I got this logged

 Error: write EPROTO C0947D4CF87F0000:error:0A0C0103:SSL routines:tls_process_key_exchange:internal
        error:../deps/openssl/openssl/ssl/statem/statem_clnt.c:2262:
        
        at WriteWrap.onWriteComplete [as oncomplete] (node:internal/stream_base_commons:94:16) {
            errno: -100,
            code: 'EPROTO',
            syscall: 'write'
        }

So I bumped the secureProtocol TLS to v1.2 and it fixes the error and works fine with nodejs 18, 16 and 12

@olivierschmid
Copy link

Same here. Thanks for this PR!

@richardkshergold
Copy link

Thanks for this - looking forward to it being merged

@cplepage
Copy link
Contributor Author

@giralte-ionic is there anything else I can do to move forward?

IT-MikeS
IT-MikeS previously approved these changes Mar 1, 2023
@IT-MikeS IT-MikeS changed the title NodeJS 18 compat fix: NodeJS 18 compat Mar 1, 2023
@IT-MikeS IT-MikeS closed this Mar 1, 2023
@IT-MikeS IT-MikeS reopened this Mar 1, 2023
@IT-MikeS IT-MikeS self-requested a review March 1, 2023 19:18
@IT-MikeS IT-MikeS dismissed their stale review March 1, 2023 19:18

needs re-review

@IT-MikeS
Copy link
Collaborator

IT-MikeS commented Mar 1, 2023

Please run npm run fmt from the root of the project and commit the changes it makes. Then the tests can run and we can see if any other errors appear from those tests.

@markemer markemer self-requested a review March 1, 2023 20:09
@markemer markemer merged commit 45050bb into ionic-team:develop Mar 2, 2023
github-actions bot pushed a commit that referenced this pull request Mar 2, 2023
## [1.7.2](v1.7.1...v1.7.2) (2023-03-02)

### Bug Fixes

* NodeJS 18 compat ([#275](#275)) ([45050bb](45050bb))
@Ionitron
Copy link
Collaborator

Ionitron commented Mar 2, 2023

🎉 This PR is included in version 1.7.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants