Page MenuHomePhabricator

Fix a bad method call signature throwing exceptions in newer Node
ClosedPublic

Authored by epriestley on Dec 10 2018, 7:43 PM.
Tags
None
Referenced Files
F15515588: D19860.id.diff
Fri, Apr 18, 11:40 AM
F15513317: D19860.diff
Thu, Apr 17, 5:59 PM
F15479054: D19860.diff
Tue, Apr 8, 5:04 AM
F15457486: D19860.diff
Sun, Mar 30, 5:10 PM
F15416050: D19860.id47423.diff
Thu, Mar 20, 8:56 AM
F15406657: D19860.id.diff
Mar 18 2025, 1:09 PM
F15401929: D19860.diff
Mar 17 2025, 7:48 PM
F15308969: D19860.id47439.diff
Mar 6 2025, 6:47 AM
Subscribers
None

Details

Summary

Ref T13222. See PHI996. Ref T10743. For context, perhaps see T12171.

Node changed some signatures, behaviors, and error handling here in recent versions. As far as I can tell:

  • The script.runInNewContext(...) method has never taken a path parameter, and passing the path has always been wrong.
  • The script.runInNewContext(...) method started taking an [options] parameter at some point, and validating it, so the bad path parameter now throws.
  • vm.createScript(...) is "soft deprecated" but basically fine, and keeping it looks more compatible.

This seems like the smallest and most compatible correct change.

Test Plan

Under Node 10, started Aphlict. Before: fatal error on bad options parameter to runInNewContext() (expected dictionary). After: notification server starts.

Diff Detail

Repository
rP Phabricator
Branch
notify1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 21276
Build 28944: Run Core Tests
Build 28943: arc lint + arc unit