Version 9.0.0#422
Open
nulltoken wants to merge 9 commits into
Open
Conversation
cf27e66 to
951ba8f
Compare
6ea5680 to
0069ffd
Compare
3 tasks
8cea1aa to
5c528ea
Compare
poveden
requested changes
Jun 11, 2026
| chunks.push(chunk); | ||
| }); | ||
| req.on('end', () => { | ||
| resolve(Buffer.concat(chunks).toString('utf8')); |
Contributor
There was a problem hiding this comment.
You are assuming that the body is text, and UTF-8 to boot. I know that most likely it will be text, but not necessarily UTF-8:
application/json: The default is UTF-8, but the client might specify otherwise.application/x-www-form-urlencoded: Technically is ASCII with BOM-less percent-encoded UTF-8.
Welcome to the world of encodings and charsets! 😜
So, to honor the "raw" in the function's name, I suggest that you return a Buffer instead of a string, and let the caller decide what to do... which will be, most surely, do toString('utf8'), but at least the intent will be clear.
| }); | ||
| } | ||
|
|
||
| return parsed as Record<string, unknown>; |
Contributor
There was a problem hiding this comment.
If parsed is an array, how does that translate to Record<string, unknown>? There are no key+value pairs in there.
| * @param err The error to handle. | ||
| * @param res The server response object. | ||
| */ | ||
| export function errorHandler(err: unknown, res: ServerResponse): void { |
Contributor
There was a problem hiding this comment.
Perhaps using Problem Details would be nicer?
For 400:
HTTP/1.1 400 Bad Request
Content-Type: application/problem+json
{
"type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
"title": "Bad Request",
"detail": "The actual error message."
}
For 500:
HTTP/1.1 500 Internal Server Error
Content-Type: application/problem+json
{
"type": "https://tools.ietf.org/html/rfc9110#section-15.6.1",
"title": "Internal Server Error",
"detail": "Most certainly a bug in the library code. Check the logs for more details and report this to the maintainers."
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
npm testlocally and all tests are passing.PR Description
service.addRoute()to expose additional routes next to OIDC endpoints. May be useful when willing to stub some vendor specific management routes for instance.