◐ Shell
clean mode source ↗

src: refactor and harden `ProcessEmitWarning()` by addaleax · Pull Request #17420 · nodejs/node

@nodejs-github-bot nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

lib / src

Issues and PRs related to general changes in the lib or src directory.

labels

Dec 1, 2017

AndreasMadsen

AndreasMadsen

bnoordhuis

AndreasMadsen

@addaleax addaleax added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Dec 9, 2017

@apapirovski apapirovski removed the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Dec 9, 2017
- Handle exceptions when getting `process.emitWarning` or when
  calling it properly
- Add `Maybe<bool>` to the return type, like the V8 API uses it
  to indicate failure conditions
- Update call sites to account for that and clean up/return to JS
  when encountering an error
- Add an internal `ProcessEmitDeprecationWarning()` sibling
  for use in nodejs#17417,
  with common code extracted to a helper function
- Allow the warning to contain non-Latin-1 characters. Since the
  message will usually be a template string containing data passed
  from the user, this is the right thing to do.
- Add tests for the failure modes (except string creation failures)
  and UTF-8 warning messages.

Refs: nodejs#17417

@addaleax addaleax added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Dec 10, 2017

BridgeAR

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request

Dec 12, 2017
- Handle exceptions when getting `process.emitWarning` or when
  calling it properly
- Add `Maybe<bool>` to the return type, like the V8 API uses it
  to indicate failure conditions
- Update call sites to account for that and clean up/return to JS
  when encountering an error
- Add an internal `ProcessEmitDeprecationWarning()` sibling
  for use in nodejs#17417,
  with common code extracted to a helper function
- Allow the warning to contain non-Latin-1 characters. Since the
  message will usually be a template string containing data passed
  from the user, this is the right thing to do.
- Add tests for the failure modes (except string creation failures)
  and UTF-8 warning messages.

PR-URL: nodejs#17420
Refs: nodejs#17417
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

@addaleax addaleax removed the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Dec 13, 2017

MylesBorins pushed a commit that referenced this pull request

Jan 8, 2018
- Handle exceptions when getting `process.emitWarning` or when
  calling it properly
- Add `Maybe<bool>` to the return type, like the V8 API uses it
  to indicate failure conditions
- Update call sites to account for that and clean up/return to JS
  when encountering an error
- Add an internal `ProcessEmitDeprecationWarning()` sibling
  for use in #17417,
  with common code extracted to a helper function
- Allow the warning to contain non-Latin-1 characters. Since the
  message will usually be a template string containing data passed
  from the user, this is the right thing to do.
- Add tests for the failure modes (except string creation failures)
  and UTF-8 warning messages.

PR-URL: #17420
Refs: #17417
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

@addaleax addaleax deleted the harden-process-emitwarning branch

January 22, 2018 14:43