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

Better inner loop experience for blocked HTTP hosts #1090

Closed
itowlson opened this issue Jan 31, 2023 · 3 comments · Fixed by #1833
Closed

Better inner loop experience for blocked HTTP hosts #1090

itowlson opened this issue Jan 31, 2023 · 3 comments · Fixed by #1833

Comments

@itowlson
Copy link
Collaborator

If Spin blocks an outbound HTTP request for not being on the allow list, it writes an info log, which nobody sees, and returns a DestinationNotAllowed error to the application, which the application probably buries in its log.

However, in the inner dev loop, the likely cause of this is that the user has forgotten to add the host to the component's allow list. It would be good if, in the local dev experience (in the terminal and running from a spin.toml file), we could provide users more specific and actionable feedback. Some possible ideas:

  • Bypass the normal logging and print a message directly to stderr, stating the component and host, and pointing them to allowed_http_hosts - natural but risks getting lost in spew
  • Collect blocked hosts and print a list as an "after action report" when the user terminates Spin - bit more complicated and provides less feedback in the moment, but very easily accessible
  • When a host is blocked, prompt the user if they want to allow it / add it to the allow list - very slick, but when I prototyped it I kept going "why isn't my app responding, oh, it's because it's waiting on a prompt over in my terminal" (and this was me knowing I was testing the prompting behaviour!)
  • A hybrid of the last two options, where you get the after action report and can go through it approving items - ooh nice, but remember at this point we have already had a Ctrl+C from the user and are meant to be tearing down the hated process
  • SPINTERM

Any other offers?

@vdice
Copy link
Contributor

vdice commented Feb 16, 2023

I'm afraid I don't have an additional approach to add, but I think I'd vote for option 1 above (though in addition to sending the DestinationNotAllowed error to the application -- but perhaps this was intended to remain anyways). I'm less keen on interactive scenarios... and unless there are other items we can already think of to go into a potential 'after-action report', it might be a bit early to add one at this stage.

@radu-matei
Copy link
Member

I think an option would be to update the DestinationNotAllowed error to include the host in question.
If we had that, together with instructions (or pointing to a URL with instructions) on how to add it, that would make the experience better IMO.

@itowlson
Copy link
Collaborator Author

itowlson commented Oct 2, 2023

Decision was for "Bypass the normal logging and print a message directly to stderr, stating the component and host, and pointing them to allowed_http_hosts"

@github-project-automation github-project-automation bot moved this from 📋 Investigating / Open for Comment to ✅ Done in Spin Triage Oct 3, 2023
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 a pull request may close this issue.

5 participants