Replace NetworkError::Internal with structured enum variants for calls with literal strings by uthmaniv · Pull Request #36599 · servo/servo

uthmaniv · 2025-04-18T12:56:46Z

Replace NetworkError::Internal with structured enum variants

Adds UnsupportedScheme,CorsViolation,ConnectionFailure,Timeout,RedirectError,InvalidMethod,ResourceError,SecurityBlock,MixedContent,CacheError,InvalidPort, LocalDirectoryError, variants in NetworkError enum.
Refactored the usage of NetworkError::Internal(String) to use the appropriate new variant

Testing: Changes does not require test.
Fixes: #36434

jdm

Good start!

components/shared/net/lib.rs

Barry-dE · 2025-04-23T19:49:30Z

@uthmaniv The PR isn't getting linked to the issue. I think removing "part of" before the issue number fixes this.

components/net/fetch/methods.rs

jdm

This is close! Just a couple more requests for clarity in the new errors.

components/net/http_loader.rs

components/shared/net/lib.rs

jdm

Let's make sure we use the new variant!

components/net/http_loader.rs

servo-wpt-sync · 2025-05-23T22:34:52Z

🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync.

TimvdLippe · 2025-12-26T12:15:19Z

@uthmaniv apologies for the delay in following up on this PR. Are you still interested in contributing it? If not, we can also resolve the merge conflicts and get it done. Thanks for helping Servo!

uthmaniv · 2025-12-26T12:19:52Z

@uthmaniv apologies for the delay in following up on this PR. Are you still interested in contributing it? If not, we can also resolve the merge conflicts and get it done. Thanks for helping Servo!

Yes I am still interested in contributing to the servo project, I will look into the PR ,update it and resolve the conflicts . Thanks

Signed-off-by: Uthman Yahaya Baba <[email protected]>

…ith granular enum variants Signed-off-by: Uthman Yahaya Baba <[email protected]>

…with granular enum variants Signed-off-by: Uthman Yahaya Baba <[email protected]>

Signed-off-by: Uthman Yahaya Baba <[email protected]>

…with granular enum variants Signed-off-by: Uthman Yahaya Baba <[email protected]>

Signed-off-by: Uthman Yahaya Baba <[email protected]>

Signed-off-by: Usman Yahaya Baba <[email protected]>

Signed-off-by: Tim van der Lippe <[email protected]>

TimvdLippe

I have addressed the two remaining comments from Josh in a commit and pushed that it to this PR. Thanks for contributing!

uthmaniv · 2026-01-01T14:56:51Z

I have addressed the two remaining comments from Josh in a commit and pushed that it to this PR. Thanks for contributing!

It's a pleasure. Thank you

Loirooriol · 2026-01-01T16:40:46Z

This is making various CSP tests crash: https://github.com/servo/servo/actions/runs/20641525515

Rather than listening all individual errors, the only case where we shouldn't do anything is `LoadCancelled`. In fact, we should return early here, since otherwise we go through the full script_thread to eventually return anyways on line 1262 since there are no page headers available. Signed-off-by: Tim van der Lippe <[email protected]>

uthmaniv mentioned this pull request Apr 18, 2025

NetworkError::Internal should use an enum instead of a string #36434

Closed

jdm requested changes Apr 21, 2025

View reviewed changes

components/shared/net/lib.rs Outdated Show resolved Hide resolved

components/shared/net/lib.rs Outdated Show resolved Hide resolved

components/shared/net/lib.rs Outdated Show resolved Hide resolved

uthmaniv force-pushed the network-error-variants branch from 7993620 to c7d205d Compare April 22, 2025 15:05

jdm reviewed Apr 22, 2025

View reviewed changes

components/shared/net/lib.rs Show resolved Hide resolved

uthmaniv force-pushed the network-error-variants branch from fac0d52 to 1b396e5 Compare April 24, 2025 00:01

uthmaniv requested a review from gterzian as a code owner April 24, 2025 00:01

uthmaniv force-pushed the network-error-variants branch 2 times, most recently from 9f4fec8 to be1c845 Compare April 24, 2025 13:51

jdm reviewed Apr 26, 2025

View reviewed changes

components/net/fetch/methods.rs Outdated Show resolved Hide resolved

jdm reviewed Apr 26, 2025

View reviewed changes

components/net/http_loader.rs Outdated Show resolved Hide resolved

components/net/http_loader.rs Outdated Show resolved Hide resolved

components/shared/net/lib.rs Outdated Show resolved Hide resolved

uthmaniv force-pushed the network-error-variants branch from 0abd71e to be1c845 Compare April 27, 2025 18:08

jdm approved these changes May 2, 2025

View reviewed changes

components/net/http_loader.rs Outdated Show resolved Hide resolved

uthmaniv force-pushed the network-error-variants branch from bd2c6f1 to d64aa28 Compare May 23, 2025 22:34

uthmaniv and others added 8 commits January 1, 2026 11:28

Replace NetworkError::Internal with structured enum variants

1e8c351

Signed-off-by: Uthman Yahaya Baba <[email protected]>

Replace NetworkError::CorsViolation and NetworkError::SecurityBlock w…

ea3eed1

…ith granular enum variants Signed-off-by: Uthman Yahaya Baba <[email protected]>

Replace NetworkError::CorsViolation and NetworkErrorr::SecurityBlock …

f9998ab

…with granular enum variants Signed-off-by: Uthman Yahaya Baba <[email protected]>

Debug Implementation for NetworkError

d825a89

Signed-off-by: Uthman Yahaya Baba <[email protected]>

Debug Implementation for NetworkError

48cbb68

Signed-off-by: Uthman Yahaya Baba <[email protected]>

Replace NetworkError::CorsViolation and NetworkErrorr::SecurityBlock …

0b87800

…with granular enum variants Signed-off-by: Uthman Yahaya Baba <[email protected]>

Resolved merge conflict

96c9c77

Signed-off-by: Uthman Yahaya Baba <[email protected]>

Resolve Conflicts

a06d848

Signed-off-by: Usman Yahaya Baba <[email protected]>

uthmaniv force-pushed the network-error-variants branch from f0eec0a to a06d848 Compare January 1, 2026 14:10

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 1, 2026

Address reviewer feedback

6455f5b

Signed-off-by: Tim van der Lippe <[email protected]>

TimvdLippe approved these changes Jan 1, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jan 1, 2026

TimvdLippe enabled auto-merge January 1, 2026 14:56

TimvdLippe added this pull request to the merge queue Jan 1, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 1, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 1, 2026

servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 1, 2026

TimvdLippe added this pull request to the merge queue Jan 1, 2026

servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 1, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 1, 2026

servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 1, 2026

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 2, 2026

TimvdLippe enabled auto-merge January 2, 2026 10:21

TimvdLippe added this pull request to the merge queue Jan 2, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 2, 2026

Merged via the queue into servo:main with commit 6decaae Jan 2, 2026
32 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 2, 2026

jdm mentioned this pull request Jan 4, 2026

Some failed requests should record resource timing entries #41667

Closed

Uh oh!

Conversation

uthmaniv commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Barry-dE commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

servo-wpt-sync commented May 23, 2025

Uh oh!

TimvdLippe commented Dec 26, 2025

Uh oh!

uthmaniv commented Dec 26, 2025

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

uthmaniv commented Jan 1, 2026

Uh oh!

Uh oh!

Uh oh!

Loirooriol commented Jan 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

uthmaniv commented Apr 18, 2025 •

edited

Loading

Barry-dE commented Apr 23, 2025 •

edited

Loading