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

RequestContext does not include Request #3964

Closed
shafreenAnfar opened this issue Jan 20, 2023 · 4 comments · Fixed by ballerina-platform/module-ballerina-http#1552
Closed
Assignees
Labels

Comments

@shafreenAnfar
Copy link
Contributor

We need to fix it.

@shafreenAnfar shafreenAnfar added Type/Improvement module/http Team/PCM Protocol connector packages related issues labels Jan 20, 2023
@TharmiganK
Copy link
Contributor

We could allow http:Request as a parameter to the interceptResponseError remote method i.e.

service class ResponseErrorInterceptor {
  *http:ResponseErrorInterceptor;

  remote function interceptResponseError(error err, http:RequestContext ctx, http:Request req) 
      returns http:NextService|error? {
    io:println(req.rawPath);
    return ctx.next();
  }
}

@shafreenAnfar will this fulfill your requirement?

@TharmiganK
Copy link
Contributor

Had a chat with @shafreenAnfar and @chamil321 and come up with the following options :

Options Pros Cons
1. Expose the Request as a parameter in the response path - Easier to implement compared to other options - This will expose unused setter methods in the response path
- This may make the RequestContext obsolete, since Request can be accessed directly in the response interceptors
2. Introduce a read-only version of the Request and expose that in the response path as a parameter - This will hide the unnecessary setter methods and will expose only the required ones in read-only mode - This can confuse the users since there are two versions of the request
3. Expose the required fields and getters of Request via RequestContext - This will hide the unnecessary setter methods and will expose only the required ones - These informations should be exposed only in response path, so accessing them in request path will not work

@shafreenAnfar
Copy link
Contributor Author

I think given that each has pros and cons, I feel the best option is 1, which results in the below.

Request path error handlers -> Request, RequestContext, error
Response path error handlers -> Response, Request, RequestContext, error

@chamil321
Copy link
Contributor

I think given that each has pros and cons, I feel the best option is 1, which results in the below.

Request path error handlers -> Request, RequestContext, error Response path error handlers -> Response, Request, RequestContext, error

+1. I also think the same. Using the request at response path error handlers are not very frequent and the impact from changing the values of the request at this level is also minimum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants