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

All CORS rejections reported to server logs #2486

Closed
tsegismont opened this issue Oct 14, 2023 · 3 comments
Closed

All CORS rejections reported to server logs #2486

tsegismont opened this issue Oct 14, 2023 · 3 comments
Assignees
Milestone

Comments

@tsegismont
Copy link
Contributor

Currently, all CORS rejections are reported to the server logs.

This is an excerpt of the Vert.x starter logs.

Oct 14 07:32:54 vertx java[535818]: 07:32:54.716 [vert.x-eventloop-thread-0] ERROR io.vertx.ext.web.RoutingContext - Unhandled exception in router
Oct 14 07:32:54 vertx java[535818]: java.lang.IllegalStateException: CORS Rejected - Invalid origin
Oct 14 07:32:54 vertx java[535818]:         at io.vertx.ext.web.handler.impl.CorsHandlerImpl.handle(CorsHandlerImpl.java:252) ~[vertx-starter-latest-fat.jar:?]
Oct 14 07:32:54 vertx java[535818]:         at io.vertx.ext.web.handler.impl.CorsHandlerImpl.handle(CorsHandlerImpl.java:41) ~[vertx-starter-latest-fat.jar:?]

There is a lot of log entries like this one.

Of course, one can set the log level to FATAL for this category. But given these rejections are expected and there is no action the user shall take, we could try to avoid logging altogether (or at least lower to DEBUG level)

@tsegismont tsegismont added this to the 4.5.0 milestone Oct 14, 2023
@tsegismont tsegismont self-assigned this Oct 14, 2023
@tsegismont
Copy link
Contributor Author

@pmlopes it seems this comes from #1857

How about replacing the context failure with exception by trace-level logs? Like this:

      context
        .response()
        .setStatusMessage("CORS Rejected - Invalid origin");
      LOG.trace("CORS Rejected - Invalid origin");
      context.fail(403);

Instead of:

      context
        .response()
        .setStatusMessage("CORS Rejected - Invalid origin");
      context
        .fail(403, new IllegalStateException("CORS Rejected - Invalid origin"));

@tsegismont
Copy link
Contributor Author

By the way, invoking context#fail even without a message or exception leads to Unhandled exception in router printed in the logs.

I'm curious why we can't send the response directly. I assumed the next handlers wouldn't be called anyway.

tsegismont added a commit to tsegismont/vertx-web that referenced this issue Oct 25, 2023
…al message

Related to vert-x3#2486
Follows-up on vert-x3#1857

ISE creates a stack trace which isn't really useful. In these cases, we only care about the message.

This change makes the application log a single line (or more if the message is long). Besides, it saves the cost of creating the ISE stack trace.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this issue Oct 25, 2023
…al message (#2496)

Related to #2486
Follows-up on #1857

ISE creates a stack trace which isn't really useful. In these cases, we only care about the message.

This change makes the application log a single line (or more if the message is long). Besides, it saves the cost of creating the ISE stack trace.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this issue Oct 25, 2023
…al message (#2496)

Related to #2486
Follows-up on #1857

ISE creates a stack trace which isn't really useful. In these cases, we only care about the message.

This change makes the application log a single line (or more if the message is long). Besides, it saves the cost of creating the ISE stack trace.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
@tsegismont
Copy link
Contributor Author

Closed by a3c1907

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

No branches or pull requests

1 participant