Skip to content

Add onError request parameter - #4378

Open
martinbonnin wants to merge 4 commits into
graphql-java:masterfrom
martinbonnin:on-error
Open

Add onError request parameter#4378
martinbonnin wants to merge 4 commits into
graphql-java:masterfrom
martinbonnin:on-error

Conversation

@martinbonnin

@martinbonnin martinbonnin commented May 11, 2026

Copy link
Copy Markdown
Contributor

I'm opening this as a heads up: see graphql/graphql-spec#1163

Not sure we want to merge is now since it's not in graphql-js yet and the @experimental_disableErrorPropagation directive still gets the job done for now. But looks like this is the direction we're taking.

@github-actions

github-actions Bot commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Test Report

Test Results

Java Version Total Passed Failed Errors Skipped
Java 11 5931 (+4 🟢) 5875 (+4 🟢) 0 (±0) 0 (±0) 56 (±0)
Java 17 5931 (+4 🟢) 5874 (+4 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 21 5931 (+4 🟢) 5874 (+4 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 25 5931 (+4 🟢) 5874 (+4 🟢) 0 (±0) 0 (±0) 57 (±0)
jcstress 32 (±0) 32 (±0) 0 (±0) 0 (±0) 0 (±0)
Total 23756 (+16 🟢) 23529 (+16 🟢) 0 (±0) 0 (±0) 227 (±0)

Code Coverage (Java 25)

Metric Covered Missed Coverage vs Master
Lines 30066 3121 90.6% ±0.0%
Branches 8791 1519 85.3% ±0.0%
Methods 8032 1206 86.9% ±0.0%

Changed Class Coverage (2 classes)

Class Line Branch Method
g.e.Execution +0.3% 🟢 +3.5% 🟢 ±0.0%
g.e.OnError +100.0% 🟢 ±0.0% +100.0% 🟢

Full HTML report: build artifact jacoco-html-report

Updated: 2026-08-14 10:36:04 UTC

@bbakerman

bbakerman commented May 31, 2026

Copy link
Copy Markdown
Member

One possible approach might be to "maintain" the old directive WHILE adding the new ExecutionInput based approach.

That way anyone who has used the old will be able to migrate to the new without breaking. This will cost time and complexity of course but it will be "less impactful" to current users

That said - at what point is the kill date? One could argue it was "experimental" and hence the kill date is immediate as this PR has done

@martinbonnin

martinbonnin commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@bbakerman agreed, I'll split this PR in 2 so we can have both at the same time.

Edit: added back @experimental_disableErrorPropagation in e91ba02, will do the PR to remove @experimental_disableErrorPropagation when needed

@martinbonnin martinbonnin changed the title Replace @experimental_disableErrorPropagation with onError parameter Add onError request parameter Jun 1, 2026
@martinbonnin martinbonnin mentioned this pull request Jun 2, 2026
Open
@martinbonnin martinbonnin mentioned this pull request Jul 22, 2026
Open
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Hello, this pull request has been inactive for 60 days, so we're marking it as stale. If you would like to continue working on this pull request, please make an update within the next 30 days, or we'll close the pull request.

@github-actions github-actions Bot added the Stale label Aug 1, 2026
@martinbonnin
martinbonnin marked this pull request as ready for review August 14, 2026 10:26
@martinbonnin

martinbonnin commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

This is still pretty much up to date. I think we should merge it.

@github-actions github-actions Bot removed the Stale label Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@martinbonnin @bbakerman