Skip to content

feat: Proper Http request handling#3298

Merged
aterga merged 12 commits intomainfrom
arshavir/ID-157
Aug 26, 2025
Merged

feat: Proper Http request handling#3298
aterga merged 12 commits intomainfrom
arshavir/ID-157

Conversation

@aterga
Copy link
Copy Markdown
Collaborator

@aterga aterga commented Aug 22, 2025

Motivation

Clients want to get more meaningful responses for OPTIONS requests.

Changes

Remove http_request_update and handle OPTIONS, GET requests via query calls.

Tests

Manual testing, cargo test

Comment thread src/internet_identity/src/main.rs Outdated
Comment thread src/internet_identity/src/http.rs
Comment thread .github/workflows/canister-tests.yml
Comment thread docs/ii-spec.mdx Outdated
Copy link
Copy Markdown
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments.

How did you test it manually?

We need to keep /faq because it's used in II 1.0

Comment thread docs/ii-spec.mdx Outdated
Comment thread src/internet_identity/src/http.rs
Comment thread src/internet_identity/src/http.rs Outdated
@aterga aterga marked this pull request as draft August 22, 2025 19:53
aterga and others added 3 commits August 22, 2025 21:57
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Comment thread src/frontend/src/hooks.ts Outdated
Comment thread src/internet_identity/src/http.rs
@aterga aterga force-pushed the arshavir/ID-157 branch 2 times, most recently from 1a8f192 to 1478950 Compare August 25, 2025 20:07
Comment thread src/internet_identity/src/http.rs
@aterga aterga marked this pull request as ready for review August 25, 2025 21:03
Comment thread .github/workflows/canister-tests.yml
Copy link
Copy Markdown
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of questions but it looks ready!

Comment thread src/internet_identity/src/http.rs
Comment thread src/internet_identity/src/http.rs Outdated
@aterga aterga requested review from lmuntaner and sea-snake August 26, 2025 08:13
Copy link
Copy Markdown
Contributor

@sea-snake sea-snake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in a call, PR keeps most implementation "as is" while adding the minimal changes required to support OPTIONS request.

@aterga aterga enabled auto-merge August 26, 2025 13:03
@aterga aterga marked this pull request as draft August 26, 2025 13:59
auto-merge was automatically disabled August 26, 2025 13:59

Pull request was converted to draft

@aterga
Copy link
Copy Markdown
Collaborator Author

aterga commented Aug 26, 2025

Converting to draft since it's supposed to be included in not the next but the second-next release

@aterga aterga marked this pull request as ready for review August 26, 2025 22:04
@aterga aterga dismissed lmuntaner’s stale review August 26, 2025 22:05

Thanks for the review, addressed all the feedback.

@aterga aterga added this pull request to the merge queue Aug 26, 2025
Merged via the queue into main with commit c8b2787 Aug 26, 2025
79 checks passed
@aterga aterga deleted the arshavir/ID-157 branch August 26, 2025 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants