Updated Server Logger and OnSession functions

[brnsampson]

Logger is a function that should return a per-request *slog.Logger

OnSession changed to remove Session and Subscription references

• Accepts (w http.ResponseWriter, r *http.Request)
• Returns (topics []string, allowed bool)

I updated the complex example and server_test.go test to account for the changes as well.

I tried to avoid altering any behavior, with the exception that if a session was accepted by the user but an empty or nil topic slice is returned we just use the default slice instead of raising a panic.

[brnsampson]

I just realized that I didn't address the user returning (nil, true) from OnSession. One sec.

Edit: actually on second thought, I think it's fine. We shouldn't really care about the exact return value of a Subscription if the use returns false from OnSession, and there isn't anything that would cause undefined behavior or nil pointer dereferences.

[tmaxmax]

We should also modify the CHANGELOG accordingly.

[tmaxmax]

Partially related to this, I've just realised something: could you also remove this line of code and add the following line in the CHANGELOG under the "Fixed" section:

- `sse.Session` doesn't write the header explicitly anymore. This would cause a `http: superfluous response.WriteHeader call` warning being logged when `sse.Server.OnSession` writes a response code itself when accepting a session. The change was initially introduced to remove the warning for users of certain external libraries (see #41) but this is the issue of the external library, not of `go-sse`. If you encounter this warning when using an external library, write the response code yourself in the HTTP handler before subscribing the `sse.Session`, as described in the linked discussion.

[brnsampson]

Sure, I'm assuming this should be under 0.11.0 in the changelog?

[tmaxmax]

Let's write "Unreleased" for now as a placeholder for the version. I write the version only when effectively releasing it.

[brnsampson]

Okay, I think most of the comment changes have been addressed.

If you are fine with me changing the structure of the example a little more I would probably lean towards eliminating the default logger, inlining the getLogger and setupLogger functions to remove them, and just putting withLogger and loggerCtxKey into main since they are not very long.

I'll have to poke around for a minute to see how to disable some formatters in my editor to change those line breaks. Currently the formmaters just run on save so whenever I try to change it they get re-formatted when I close the file. I think it also made some very small formatting changes to CHANGELOG.md, but I think that may just have been whitespace related.

[tmaxmax]

Sorry for the barrage of comments on the changelog – I haven't stated the expectations clearly. You should be able to just commit those suggestions from the GitHub UI and now waste any time with it.

[brnsampson]

I have a few things to take care of, so I have to finish this a bit later but I'll change the changelog soon

[tmaxmax]

I've committed the suggested CHANGELOG adjustments and other nitpicks myself, to not waste time on those. Open comments left:

• a discussion on the documentation of withLogger
• some CHANGELOG formatting issues
• a discussion on getLogger

Apart from that everything's good to go.

[brnsampson]

Apologies for the delay, we had some cat health drama that kept me busy. It was also far, far more involved to track down how to disable a builtin formatter for one file than I expected for some reason.

I think I got to all the comments, so let me know if there is anything else. I don't mind rebasing the whole branch into a single commit or if you want to squash and merge that's fine too.

[tmaxmax]

Hi, everything looks great! Don't worry about any delay, code's not going anywhere. And, as always, family first 😌

Rebasing would be perfect! There are also some minor linter issues to fix. For one of them just add the value hugeParam in .golangci.yml in the linter-settings.gocritic.disabled-checks list.

[brnsampson]

I haven't been able to get golangci-lint to work locally so I wasn't able to verify that it will run cleanly, but I added the exclusion and squashed everything down to 1 commit

[tmaxmax]

these were the other 2 linter comments, i believe

[brnsampson]

Sorry I had to disappear again at the end, it has been a crazy week. Thanks for wrapping it up at the end!