Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the uri property of RedirectException nullable #49045

Closed
brianquinlan opened this issue May 17, 2022 · 8 comments
Closed

Make the uri property of RedirectException nullable #49045

brianquinlan opened this issue May 17, 2022 · 8 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. enhancement-breaking-change An enhancement which is breaking. library-_http

Comments

@brianquinlan
Copy link
Contributor

Change

Change:

class RedirectException implements HttpException {
-  Uri get uri => ...
+  Uri? get uri => ...
}

Rationale

This will allow us to cleanly fix #49012, which causes a StateError when there is no available uri.

Impact

This will only break code that uses the RedirectException class directly and does not handle the null uri case.

Code that works with the HttpException base class will not need to be changed because uri is already nullable in that class.

This change does not break any Google code or any third-party code used by Google.

Mitigation

Users must handle a possible null return from RedirectException.uri.

@brianquinlan brianquinlan added library-_http enhancement-breaking-change An enhancement which is breaking. labels May 17, 2022
@brianquinlan brianquinlan self-assigned this May 17, 2022
@mit-mit mit-mit added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label May 18, 2022
@a-siva
Copy link
Contributor

a-siva commented May 18, 2022

//cc @lrhn

@itsjustkevin
Copy link
Contributor

@vsmenon @Hixie @grouma Could you all take a look at this breaking change proposal?

@grouma
Copy link
Member

grouma commented May 23, 2022

I didn't find any usages of RedirectException.uri inside Google3. I assume this doesn't impact HttpException which has the non nullable requestUrl member. This is referenced in a handful of places.

@brianquinlan
Copy link
Contributor Author

@grouma If you are referring to the HttpException in ...xbid/ui/common/exceptions/lib/exceptions.dart then that class is in a completely different class hierarchy.

@Hixie
Copy link
Contributor

Hixie commented May 27, 2022

I have no opinion on this change.

@brianquinlan
Copy link
Contributor Author

@vsmenon It's all up to you ;-)

@vsmenon
Copy link
Member

vsmenon commented Jun 2, 2022

lgtm

@brianquinlan
Copy link
Contributor Author

Fixed in e5af733

copybara-service bot pushed a commit that referenced this issue Jun 9, 2022
Bug: #49045
Change-Id: I9ac976545c0505a20a90f12333a424d0ed8cf96f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247820
Commit-Queue: Brian Quinlan <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. enhancement-breaking-change An enhancement which is breaking. library-_http
Projects
None yet
Development

No branches or pull requests

7 participants