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

Update response-timeline-viewer to show response body. #4346

Merged
merged 8 commits into from
Feb 24, 2022
Merged

Update response-timeline-viewer to show response body. #4346

merged 8 commits into from
Feb 24, 2022

Conversation

p-m-j
Copy link
Contributor

@p-m-j p-m-j commented Jan 7, 2022

Whenever I have an issue with oauth in insomnia it takes an awful lot of effort to work out what has actually gone wrong.

Have to make use of debugging proxies, or add breakpoints in dev tools.

Adding the following (or similar if this isn't appropriate) would make life a lot easier for me.

image

image

changelog(Improvements): Insomnia will now show the response body for OAuth2 requests when there's an error

@filfreire filfreire added the insomnia-stream a good candidate to look at during the weekly livestream (see #stream on https://chat.insomnia.rest) label Feb 8, 2022
@dimitropoulos
Copy link
Contributor

Hi @p-m-j

We took a look at this PR today on the Insomnia Stream

Screenshot_20220215_142546

Please feel free to verify, but I think we found a way to do exactly what you want while avoiding another problem that would come if we put the response body in the timeline at all times.

@gatzjames @jackkav @DMarby please take a look if you can. https://github.com/schrodingersket/keycloak-oauth2-ssl-testbed will be a good thing to set up while you do, but check out the stream if you want to see how we tested it. Basically you can just follow the instructions and just give keycloak a bad passoword to see an error.

@p-m-j
Copy link
Contributor Author

p-m-j commented Feb 15, 2022

Hey @dimitropoulos, Would love to have a gander but that stream link just takes me to the slack "Sign in to your workspace" page.

@dimitropoulos
Copy link
Contributor

BEFORE AFTER
Screenshot_20220215_145204 Screenshot_20220215_145049

I also put in a tiny commit to make the response timeline debug output more easily accessible. before, you had to click on that little bug icon and literally nothing else. in my opinion, it was effectively hidden before, so felt like as good a time as any to fix that.

@dimitropoulos
Copy link
Contributor

@p-m-j that's because it's a link to a channel in the insomnia community chat. You can sign up here.

In case you don't want to: here is a timestamped link to the YouTube video for the stream. We're still in early-days with the stream, so apologies for the poor bitrate (we're evaluating tools to use for this (it's only the second stream we've done, the first being just last week) and all of them require you to pay to get any better quality).

@p-m-j
Copy link
Contributor Author

p-m-j commented Feb 15, 2022

Couldn't sign up there but found https://chat.insomnia.rest/ which sends out an invite.

Cheers

@dimitropoulos
Copy link
Contributor

ohh, interesting.. hmm.. I'll have to learn how that all works, then. thanks!

Comment on lines +244 to +250
const body: ResponseTimelineEntry[] = showBody ? [
{
name: LIBCURL_DEBUG_MIGRATION_MAP.DataOut,
timestamp: Date.now(),
value: fs.readFileSync(bodyPath).toString(),
},
] : [];
Copy link
Contributor

@dimitropoulos dimitropoulos Feb 17, 2022

Choose a reason for hiding this comment

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

@jackkav since you were asking: this body variable, right here, is the core of what this PR adds. Everything else is machinery leading up to this one change.

Copy link
Member

@filfreire filfreire left a comment

Choose a reason for hiding this comment

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

@dimitropoulos marking this one as approved - since for the most part we checked it during the stream - only left 1 comment bellow.

Best to wait for feedback from either @gatzjames @jackkav @DMarby before merge.

packages/insomnia-app/app/models/response.ts Show resolved Hide resolved
@dimitropoulos dimitropoulos merged commit 41a2fd6 into Kong:develop Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
insomnia-stream a good candidate to look at during the weekly livestream (see #stream on https://chat.insomnia.rest)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants