[native] Avoid invariant crash in getDevNodeServerURL
ClosedPublic
Actions

Authored by ashoat on Jan 21 2023, 1:01 PM.

Details

Reviewers

bartek

Commits

rCOMM1e4672088b22: [native] Avoid invariant crash in getDevNodeServerURL

Summary

D6269 updated release builds to hit getDevNodeServerURL, which is causing a crash. This is happening because the result of getDevNodeServerURL is included in the default list of server options in the Developer tools section of the app.

There's probably no risk to calling this in production, especially since we already call dev-hostname.js code from release builds in natNodeServer. It's better to improve readability of the code and remove this invariant than to sidestep it.

Test Plan

Confirm release build no longer crashes

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Jan 21 2023, 1:01 PM

Herald added subscribers: atul, tomek. · View Herald TranscriptJan 21 2023, 1:02 PM

ashoat mentioned this in D6269: [native] Fix server list in Dev tools menu.Jan 21 2023, 1:02 PM

ashoat added inline comments.

native/utils/url-utils.js
43	This isn't technically needed, but I figured it's okay philosophically

Harbormaster completed remote builds in B15516: Diff 21163.Jan 21 2023, 1:16 PM

ashoat requested review of this revision.Jan 21 2023, 1:16 PM

My mistake, thanks for fixing that!
IIRC on prod the IP defaults to 192.168.1.1 (as the value from expo config will be null) so this shouldn't be harmful.

This revision is now accepted and ready to land.Jan 21 2023, 1:26 PM

Closed by commit rCOMM1e4672088b22: [native] Avoid invariant crash in getDevNodeServerURL (authored by ashoat). · Explain WhyJan 21 2023, 2:13 PM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMM1e4672088b22: [native] Avoid invariant crash in getDevNodeServerURL.