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

Trouble getting /report calls going to the correct end point #4539

Closed
Jakob37 opened this issue Mar 28, 2024 · 19 comments · Fixed by #4553
Closed

Trouble getting /report calls going to the correct end point #4539

Jakob37 opened this issue Mar 28, 2024 · 19 comments · Fixed by #4553
Labels

Comments

@Jakob37
Copy link
Collaborator

Jakob37 commented Mar 28, 2024

Hi again!

Following up on an issue previously encountered during testing of the PR: #4529

It initially looked like a CORS error, but at least a part of it seems to be a more straight forward issue. It might simply be a configuration issue on my side.

I have Scout and Chanjo2 running on two URLs. Let's call them http://scout and http://chanjo2

Navigating to each of these individually I successfully enter Scout and Chanjo2 start pages.

Now, let's say I am in a Scout case, preparing to view its Chanjo2 report. The case URL:

http://scout/cust000/643594

I can successfully access Chanjo2 for the case, and I do then end up on an URL in the following format:

http://scout/cust000/643594/chanjo2_report/report?panel_name=Test+panel+(1.0)

The issue is that when doing further requests to update coverage settings within Chanjo2, calls are made to "/report" which leads me back to:

http://scout/report

The correct location would (I guess) be http://chanjo2/report

I also had an issue with these fetch requests being upgraded to https (through <meta http-equiv="Content-Security-Policy" content="upgrade-insecure-requests"> I think). I removed that for now for testing, but might be multiple steps to take to solve this.

Thankful for any input! I'll meanwhile continue testing.

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Mar 28, 2024

I have also tried running this behind an nginx reverse proxy with end points http://scout and http://scout/chanjo2. Nginx runs the two fine, but does not resolve the issue here.

@northwestwitch
Copy link
Member

I see why this happens. At the moment Scout collects the contents of the reports from chanjo2 and returns them in a page that is within the scout context. We'd probably need to just link out to the chanjo2 app instead.

I did it this way because our clinicians want to print coverage stats inside the case report, so a function that just grabbed the contents from chanjo2 made sense. but of course reports need to be modified on the fly. I'll fix!

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Mar 28, 2024

I see why this happens. At the moment Scout collects the contents of the reports from chanjo2 and returns them in a page that is within the scout context. We'd probably need to just link out to the chanjo2 app instead.

I did it this way because our clinicians want to print coverage stats inside the case report, so a function that just grabbed the contents from chanjo2 made sense. but of course reports need to be modified on the fly. I'll fix!

OK I see! Thanks!

@northwestwitch
Copy link
Member

I'm moving this issue to Scout!

@northwestwitch northwestwitch transferred this issue from Clinical-Genomics/chanjo2 Mar 28, 2024
@northwestwitch
Copy link
Member

Hello @Jakob37. I've created a fix to this issue both in Scout(fix #4553) and chanjo2 (Clinical-Genomics/chanjo2#273). When you have time could you try the fix by using these 2 branches in combo?

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 15, 2024

Hello @Jakob37. I've created a fix to this issue both in Scout(fix #4553) and chanjo2 (Clinical-Genomics/chanjo2#273). When you have time could you try the fix by using these 2 branches in combo?

That sounds great @northwestwitch ! I'll have time to look at it early this week.

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 15, 2024

Hmm, it seems to be partially working in that the base URL now switches to Chanjo2 from Scout when clicking the outgoing links.

But I have other issues. Unsure whether due to our setup or something else. Maybe you can help me understand.

First when trying it out I got an SSL error (same as I got last time testing). This seems to be due to us still running http (we are working on this, hopefully we will switch all servers to https soon).

Anyway, I tried commenting out this line in base-layout.html, as this seemed to be interferring with us running http. (This might be the cause of my new errors).

<!-- <meta http-equiv="Content-Security-Policy" content="upgrade-insecure-requests"> -->  

Now when going to the page we get different errors. Here are the set in Chrome for the demo dataset.

The first error "Failed to load resource" comes up on page load. The second error when clicking the "Customize" dropdown for the first time. Clicking the dropdown a second time it opens.

When then changing cutoff and clicking the "Update" button, I get the final two errors (from submitAsJson in report:419).

chanjo2_april

I tried copying the data dict from the networking tab to try it in curl:

curl -X POST http://mtlucmds2.lund.skane.se:8818/report -H "Content-Type: application/json" -d @payload.json

With payload.json being a file as such:

{"build":"GRCh37","completeness_thresholds":[10,15,20,50,100],"ensembl_gene_ids":null,"hgnc_gene_ids":[170,188,338,494,64,...],"hgnc_gene_symbols":null,"interval_type":"transcripts","panel_name":"Test panel (1.0)","default_level":"15","case_display_name":"643594","samples":[{"name":"NA12882","case_name":"643594","coverage_file_path":"/home/<user>/app/scout/demo/ADM1059A2.d4","analysis_date":"2016-10-12 14:00:46+00:00"},{"name":"NA12877","case_name":"643594","coverage_file_path":"/home/<user>/app/scout/demo/ADM1059A1.d4","analysis_date":"2016-10-12 14:00:46+00:00"},{"name":"NA12878","case_name":"643594","coverage_file_path":"/home/<user>/app/scout/demo/ADM1059A3.d4","analysis_date":"2016-10-12 14:00:46+00:00"}]}

This yields an error saying that all fields are missing (which seem to obviously be there):

{
  "detail": [
    {
      "type": "missing",
      "loc": [
        "body",
        "build"
      ],
      "msg": "Field required",
      "input": null,
      "url": "https://errors.pydantic.dev/2.5/v/missing"
    },
    {
      "type": "missing",
      "loc": [
        "body",
        "interval_type"
      ],
      "msg": "Field required",
      "input": null,
      "url": "https://errors.pydantic.dev/2.5/v/missing"
    },
    {
      "type": "missing",
      "loc": [
        "body",
        "default_level"
      ],
      "msg": "Field required",
      "input": null,
      "url": "https://errors.pydantic.dev/2.5/v/missing"
    },
    {
      "type": "missing",
      "loc": [
        "body",
        "panel_name"
      ],
      "msg": "Field required",
      "input": null,
      "url": "https://errors.pydantic.dev/2.5/v/missing"
    },
    {
      "type": "missing",
      "loc": [
        "body",
        "case_display_name"
      ],
      "msg": "Field required",
      "input": null,
      "url": "https://errors.pydantic.dev/2.5/v/missing"
    },
    {
      "type": "missing",
      "loc": [
        "body",
        "samples"
      ],
      "msg": "Field required",
      "input": null,
      "url": "https://errors.pydantic.dev/2.5/v/missing"
    }
  ]
}

I guess this might be the source of the 422 error.

Do you understand what is happening here? Do you think it is related to us not running https yet, or something else?

@northwestwitch
Copy link
Member

Regarding the SSL error, yes, I think it's something that should be fixed in your server settings (we don't see it in out setup at least..).

I'm a bit lost on the following errors, as I don't understand where that submitAsJson comes from. The last changes I made in scout and chanjo2 would submit the form data as application/x-www-form-urlencoded data and not as json. Even so, json should be an accepted format when you create report data.

I need to investigate further 🤔
Thanks for testing!

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 15, 2024

Regarding the SSL error, yes, I think it's something that should be fixed in your server settings (we don't see it in out setup at least..).

OK I see. Yes, I think the SSL errors are due to us running http and the requests being upgraded due to the upgrade-insecure-requests directive in that HTML tag. But I think https will be fixed soon for us, making it a non-issue.

@Jakob37 ahhhhh -----> https://github.com/Clinical-Genomics/chanjo2/blob/07c42bf8de5742854f54b10d28369a25f8c611d7/src/chanjo2/templates/report.html#L290

Hmm, did you figure it out?

@northwestwitch
Copy link
Member

Hmm, did you figure it out?

Not yet, but I'll continue tomorrow!

@northwestwitch
Copy link
Member

northwestwitch commented Apr 16, 2024

Are you using the latest versions of #4553 and the chanjo2 branch?

On our stage server, when I click on a report link from scout:

image

I should fix the 2 errors but they don't seem to have an impact on the report creation for me.

If I for instance modify the threshold cutoff and remove some genes from the query by using the customize panel, the statistics change and I get once again the illegal invocation error, but not the POST request error you described in your comment above 🤔

image

@northwestwitch
Copy link
Member

I tried copying the data dict from the networking tab to try it in curl:

curl -X POST http://mtlucmds2.lund.skane.se:8818/report -H "Content-Type: application/json" -d @payload.json

With payload.json being a file as such:

{"build":"GRCh37","completeness_thresholds":[10,15,20,50,100],"ensembl_gene_ids":null,"hgnc_gene_ids":[170,188,338,494,64,...],"hgnc_gene_symbols":null,"interval_type":"transcripts","panel_name":"Test panel (1.0)","default_level":"15","case_display_name":"643594","samples":[{"name":"NA12882","case_name":"643594","coverage_file_path":"/home/<user>/app/scout/demo/ADM1059A2.d4","analysis_date":"2016-10-12 14:00:46+00:00"},{"name":"NA12877","case_name":"643594","coverage_file_path":"/home/<user>/app/scout/demo/ADM1059A1.d4","analysis_date":"2016-10-12 14:00:46+00:00"},{"name":"NA12878","case_name":"643594","coverage_file_path":"/home/<user>/app/scout/demo/ADM1059A3.d4","analysis_date":"2016-10-12 14:00:46+00:00"}]}

Tried with a json file on our server

{
  "build": "GRCh37",
  "completeness_thresholds": [
    10,
    15,
    20,
    50,
    100
  ],
  "ensembl_gene_ids": [],
  "hgnc_gene_ids": [
    20, 869, 1097, 3036, 3049, 3052, 3226, 4137, 5173, 6207, 6840, 9644, 10898, 11779, 12422, 15520, 16002, 17637, 18337, 18483, 19274, 23639, 24856, 28927, 28929
  ],
  "hgnc_gene_symbols": [],
  "interval_type": "transcripts",
  "default_level": 10,
  "panel_name": "Custom panel",
  "case_display_name": "",
  "samples": [
    {
      "name": "ADM1309A1",
      "coverage_file_path": "/home/proj/stage/housekeeper-bundles/safeguinea/2023-06-26/ADM997A3_lanes_12_sorted_md.d4",
      "case_name": "Test",
      "analysis_date": "2024-03-27T08:55:26.538826"
    }
  ]
}

After deploying chanjo2 branch 273

curl https://chanjo2-stage.scilifelab.se/report -H "Content-Type: application/json" -d @test.json

It returns the HTML code of the report page

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 16, 2024

I'll double check!

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 16, 2024

Looks like I hadn't pulled the latest version of the Chanjo2 branch, sorry about that. Will re-try right away.

@northwestwitch
Copy link
Member

Looks like I hadn't pulled the latest version of the Chanjo2 branch, sorry about that. Will re-try right away.

No worries :)

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 16, 2024

Aha, nice, now it seems to work!

Both changing transcripts and the coverage thresholds change the values displayed.

A minor detail is that the coverage threshold is reset to "10x" in the dropdown, but the header at the page is updated anyway so looks like it works like it should.

Before

chanjo2_1_2

After changing to 20x and just a handful of IDs

chanjo2_1_1

@Jakob37
Copy link
Collaborator Author

Jakob37 commented Apr 16, 2024

Great job 💪

@northwestwitch
Copy link
Member

Nice!! 🥳

A minor detail is that the coverage threshold is reset to "10x" in the dropdown, but the header at the page is updated anyway so looks like it works like it should.

I noticed as well. I've opened an issue in chanjo2 earlier today --> Clinical-Genomics/chanjo2#280

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants