8000 feat(core): add `context.http` and move `statusCode` there by vonagam · Pull Request #2496 · feathersjs/feathers · GitHub
[go: up one dir, main page]

Skip to content

feat(core): add context.http and move statusCode there#2496

Merged
daffl merged 1 comm 8000 it intofeathersjs:dovefrom
vonagam:feat-context-http
Nov 26, 2021
Merged

feat(core): add context.http and move statusCode there#2496
daffl merged 1 commit intofeathersjs:dovefrom
vonagam:feat-context-http

Conversation

@vonagam
Copy link
Member
@vonagam vonagam commented Nov 25, 2021

This is context.http from #2484.

Here it contains only statusCode (can be renamed to status or not, don't have an opinion one way or another so kept previous name), with the idea to add responseHeaders later (redirect is not needed since it will be covered by those two).

I used getter/setter to prevent having two sources of truth and things getting out of sync. Also ideally a depreciation message should be added to those accessors that will write a warning first time one of them is accessed (a helper function for such depreciations can be added to @feathersjs/commons).

Copy link
Member
@daffl daffl left a comment

Choose a reason for hiding this comment

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

This looks good, thank you! Just two small somments.

@daffl daffl merged commit b701bf7 into feathersjs:dove Nov 26, 2021
@BenPortner
Copy link

@daffl @vonagam this PR deprecates the Context.statusCode property and refers to http.statusCode instead. However, there is not http property for websocket transport. How are websocket statusCodes to be handled in the future?

@daffl
Copy link
Member
daffl commented Apr 17, 2025

You can still set the status code, however it will be ignored for websockets since they have no effect. Feathers errors will still be converted with their respective error statuses though.

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

0