Skip to content

src: use correct way to signal interceptor error#54418

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
targos:interceptor-error
Aug 24, 2024
Merged

src: use correct way to signal interceptor error#54418
nodejs-github-bot merged 1 commit intonodejs:mainfrom
targos:interceptor-error

Conversation

@targos
Copy link
Copy Markdown
Member

@targos targos commented Aug 17, 2024

Refs:

node/deps/v8/include/v8-function-callback.h

Lines 398 to 408 in e4f61de

void ReturnValue<T>::Set(const Local<S> handle) {
// "V8_DEPRECATE_SOON" this method if |T| is |void|.
#ifdef V8_IMMINENT_DEPRECATION_WARNINGS
static constexpr bool is_allowed_void = false;
static_assert(!std::is_void<T>::value,
"ReturnValue<void>::Set(const Local<S>) is deprecated. "
"Do nothing to indicate that the operation succeeded or use "
"SetFalse() to indicate that the operation failed (don't "
"forget to handle info.ShouldThrowOnError()). "
"See http://crbug.com/348660658 for details.");
#else

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 17, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 17, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.32%. Comparing base (e4f61de) to head (b196b6c).
Report is 50 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54418      +/-   ##
==========================================
- Coverage   87.33%   87.32%   -0.01%     
==========================================
  Files         648      648              
  Lines      182321   182321              
  Branches    34971    34973       +2     
==========================================
- Hits       159222   159217       -5     
- Misses      16374    16377       +3     
- Partials     6725     6727       +2
Files Coverage Δ
src/node_webstorage.cc 73.64% <100.00%> (ø)

... and 17 files with indirect coverage changes

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2024
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 17, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/61182/

@targos targos mentioned this pull request Aug 24, 2024
Closed
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2024
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 24, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/61404/

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 24, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/61408/

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 24, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/61411/

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 24, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 24, 2024
@nodejs-github-bot nodejs-github-bot merged commit c062b52 into nodejs:main Aug 24, 2024
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 24, 2024

Landed in c062b52

@targos targos deleted the interceptor-error branch August 24, 2024 17:10
RafaelGSS pushed a commit that referenced this pull request Aug 25, 2024
@targos @RafaelGSS
src: use correct way to signal interceptor error
88d789a 
PR-URL: #54418
Refs: https://github.com/nodejs/node/blob/e4f61de14f8cfb83f1ce0ad1597b86278cd5f5f1/deps/v8/include/v8-function-callback.h#L398-L408
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Cheng Zhao <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Aug 25, 2024
Merged
@targos targos added dont-land-on-v18.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Aug 26, 2024
@RafaelGSS RafaelGSS mentioned this pull request Aug 30, 2024
Merged
@aduh95 aduh95 mentioned this pull request Oct 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zcbenz zcbenz zcbenz approved these changes

@benjamingr benjamingr benjamingr approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@targos @nodejs-github-bot @zcbenz @benjamingr