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
F13136690: D19860.id47439.diff
Thu, May 2, 3:10 PM
Unknown Object (File)
Wed, May 1, 12:21 PM
Unknown Object (File)
Wed, May 1, 2:15 AM
Unknown Object (File)
Mon, Apr 29, 6:18 PM
Unknown Object (File)
Fri, Apr 26, 1:05 AM
Unknown Object (File)
Thu, Apr 25, 1:33 AM
Unknown Object (File)
Sat, Apr 20, 12:49 AM
Unknown Object (File)
Fri, Apr 19, 8:09 PM
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