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
F18601354: D19860.diff
Sat, Sep 13, 12:16 PM
F18574401: D19860.id47439.diff
Wed, Sep 10, 10:37 AM
F18553138: D19860.id47439.diff
Mon, Sep 8, 2:35 PM
F18515755: D19860.id.diff
Fri, Sep 5, 11:52 AM
F18509989: D19860.id.diff
Fri, Sep 5, 3:43 AM
F18503592: D19860.diff
Thu, Sep 4, 11:13 PM
F18497831: D19860.diff
Thu, Sep 4, 6:15 PM
F18345985: D19860.diff
Tue, Aug 26, 12:31 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable