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

Confusing error when request is targetted for an invalid tenant id #1792

Closed
lmsurpre opened this issue Dec 4, 2020 · 1 comment · Fixed by #1857
Closed

Confusing error when request is targetted for an invalid tenant id #1792

lmsurpre opened this issue Dec 4, 2020 · 1 comment · Fixed by #1857
Assignees
Labels
bug Something isn't working

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Dec 4, 2020

Describe the bug
Requests for an invalid tenant are coming back with a response like this:

{"resourceType":"OperationOutcome","id":"c0-a8-1-da-7c8d79ec-4bc3-4d6b-940b-d755e1bb4f24","issue":[{"severity":"fatal","code":"exception","details":{"text":"FHIRPersistenceException: fhirServer/persistence/factoryClassname is configured incorrectly"}}]}

To Reproduce
Steps to reproduce the behavior:

  1. deploy the server with the default connfig
  2. make a request with a header like X-FHIR-TENANT-ID: dummy

Expected behavior
The request should come back with a clear error message that states that the tenant wasn't valid

Additional context
In #639 we decided that all tenants should have their own datasources (and should never "fall back" to the default one).
Based on this, we should be able to check for the presence of a tenant up front (based on the existence of the corresponding tenant config directory). Maybe we should put it right in the FHIRRestServletFilter?

@lmsurpre lmsurpre added the bug Something isn't working label Dec 4, 2020
@prb112
Copy link
Contributor

prb112 commented Dec 7, 2020

Describe the bug
Requests for an invalid tenant are coming back with a response like this:

{"resourceType":"OperationOutcome","id":"c0-a8-1-da-7c8d79ec-4bc3-4d6b-940b-d755e1bb4f24","issue":[{"severity":"fatal","code":"exception","details":{"text":"FHIRPersistenceException: fhirServer/persistence/factoryClassname is configured incorrectly"}}]}

To Reproduce
Steps to reproduce the behavior:

1. deploy the server with the default connfig

2. make a request with a header like `X-FHIR-TENANT-ID: dummy`

Expected behavior
The request should come back with a clear error message that states that the tenant wasn't valid

Additional context
In #639 we decided that all tenants should have their own datasources (and should never "fall back" to the default one).
Based on this, we should be able to check for the presence of a tenant up front (based on the existence of the corresponding tenant config directory). Maybe we should put it right in the FHIRRestServletFilter?

I hit a similar issue with an old derby db in place

at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:836)
Caused by: com.ibm.fhir.persistence.exception.FHIRPersistenceException: Unexpected exception while creating JDBC persistence layer:   [probeId=c0-a8-56-14-550264d1-9a69-4f98-a7db-c1956d072bfe]
	at com.ibm.fhir.persistence.jdbc.FHIRPersistenceJDBCFactory.getInstance(FHIRPersistenceJDBCFactory.java:32)
	at com.ibm.fhir.persistence.helper.FHIRPersistenceHelper.getFHIRPersistenceImplementation(FHIRPersistenceHelper.java:86)
	... 64 more
Caused by: java.lang.IllegalStateException: createCache failed
	at com.ibm.fhir.persistence.jdbc.cache.FHIRPersistenceJDBCTenantCache.getCacheForTenantAndDatasource(FHIRPersistenceJDBCTenantCache.java:41)
	at com.ibm.fhir.persistence.jdbc.FHIRPersistenceJDBCFactory.getInstance(FHIRPersistenceJDBCFactory.java:29)
	... 65 more

@lmsurpre lmsurpre added this to the Sprint 2021-1 milestone Dec 15, 2020
@prb112 prb112 self-assigned this Jan 8, 2021
prb112 added a commit that referenced this issue Jan 8, 2021


- Disambiguates the Error Messages that are bubbled up through the
Persistence Layer
- Add Integration Test

Signed-off-by: Paul Bastide <[email protected]>
prb112 added a commit that referenced this issue Jan 8, 2021


1 - added layer at the rest level

Signed-off-by: Paul Bastide <[email protected]>
JohnTimm added a commit that referenced this issue Jan 21, 2021
* ci: introduce integration tests for fhir-audit

Signed-off-by: Paul Bastide <[email protected]>

* ci: introduce integration tests for fhir-audit

Signed-off-by: Paul Bastide <[email protected]>

* ci: add integration tests for fhir-audit feature and fix one bug with use of/from

Signed-off-by: Paul Bastide <[email protected]>

* fix: pseudo tty

Signed-off-by: Paul Bastide <[email protected]>

* fix: small change to the timeout length to 120s

Signed-off-by: Paul Bastide <[email protected]>

* fix: alternative method for getting the results from the kafka-1 container

Signed-off-by: Paul Bastide <[email protected]>

* fix: alternative method for getting the results from the kafka-1 container

Signed-off-by: Paul Bastide <[email protected]>

* fix: add get_results.sh creation of the directory

Signed-off-by: Paul Bastide <[email protected]>

* fix: update

Signed-off-by: Paul Bastide <[email protected]>

* fix: update

Signed-off-by: Paul Bastide <[email protected]>

* fix: for privileged execution

Signed-off-by: Paul Bastide <[email protected]>

* fix: difference running ci local and remote

Signed-off-by: Paul Bastide <[email protected]>

* fix: audit

Signed-off-by: Paul Bastide <[email protected]>

* fix: audit with docker copy

Signed-off-by: Paul Bastide <[email protected]>

* fix: adding tty support and stdin support

Signed-off-by: Paul Bastide <[email protected]>

* fix: work around tty issue

Signed-off-by: Paul Bastide <[email protected]>

* removing the tty references -it

Signed-off-by: Paul Bastide <[email protected]>

* changed the execution pattern

Signed-off-by: Paul Bastide <[email protected]>

* changed the execution pattern

Signed-off-by: Paul Bastide <[email protected]>

* fix: update to predefine output file

Signed-off-by: Paul Bastide <[email protected]>

* fix: update to predefine output file

Signed-off-by: Paul Bastide <[email protected]>

* fix permissions

Signed-off-by: Paul Bastide <[email protected]>

* fix permissions

Signed-off-by: Paul Bastide <[email protected]>

* fix permissions

Signed-off-by: Paul Bastide <[email protected]>

* fix permissions

Signed-off-by: Paul Bastide <[email protected]>

* fix permissions

Signed-off-by: Paul Bastide <[email protected]>

* fix permissions

Signed-off-by: Paul Bastide <[email protected]>

* issues #1839 and #1743 - support search parameter disambiguation

1. Update ParametersMap to support storing multiple search parameters
with the same code
2. Address #1743 by collecting to a map instead of a list
3. Update SearchUtil.getSearchParameter to lookup the search parameter
by URI from the config if possible (instead of applying the filter to
the full set of built-in parameters).
4. Update the docs to reflect that search parameter filtering now
applies to tenant-specific search parameters as well. This should help
us move toward #1596

Also fixed a bad trace message and did some minor formatting / javadoc.

Signed-off-by: Lee Surprenant <[email protected]>

* ci: work around issue with tty

Signed-off-by: Paul Bastide <[email protected]>

* fix: test that doesn't account for year shifts

Signed-off-by: Paul Bastide <[email protected]>

* fix: test that doesn't account for year shifts

Signed-off-by: Paul Bastide <[email protected]>

* ci: work around issue with tty

Signed-off-by: Paul Bastide <[email protected]>

* remove hardcoded year from SearchLastUpdatedIdTest

Signed-off-by: Lee Surprenant <[email protected]>

* fix: change the integration pattern slightly for tty

Signed-off-by: Paul Bastide <[email protected]>

* fix: change the integration pattern slightly for tty

Signed-off-by: Paul Bastide <[email protected]>

* fix: change the integration pattern slightly for tty

Signed-off-by: Paul Bastide <[email protected]>

* fix: last two tests to update with dynamic year

Signed-off-by: Paul Bastide <[email protected]>

* issues #1839 and #1743 - support search parameter disambiguation

1. Update ParametersMap to support storing multiple search parameters
with the same code
2. Address #1743 by collecting to a map instead of a list
3. Update SearchUtil.getSearchParameter to lookup the search parameter
by URI from the config if possible (instead of applying the filter to
the full set of built-in parameters).
4. Update the docs to reflect that search parameter filtering now
applies to tenant-specific search parameters as well. This should help
us move toward #1596

Also fixed a bad trace message and did some minor formatting / javadoc.

Signed-off-by: Lee Surprenant <[email protected]>

* Apply suggestions from code review

Signed-off-by: Lee Surprenant <[email protected]>

* Update build/audit/README.md

Signed-off-by: Paul Bastide <[email protected]>

Co-authored-by: Lee Surprenant <[email protected]>

* add info on accessing the bulk operation job logs

Signed-off-by: Lee Surprenant <[email protected]>

* Add unit tests for the ParametersMap

Also made a minor change to insertAll so it gets the code from the
existing ParametersMap instead of from the SearchParameters in the map.
Usually these are the same, but they can differ.

Signed-off-by: Lee Surprenant <[email protected]>

* Issue #1849 - AuthZ interceptor validate/convert search requests

Signed-off-by: Mike Schroeder <[email protected]>

* Issue #1849 - address review comments

Signed-off-by: Mike Schroeder <[email protected]>

* Duplicate Job Parameters fhir.dataSourcesInfo are created #1855

- Removed the duplicate serialization of the fhir.dataSourcesInfo

Signed-off-by: Paul Bastide <[email protected]>

* Confusing error when request is targetted for an invalid tenant id #1792

- Disambiguates the Error Messages that are bubbled up through the
Persistence Layer
- Add Integration Test

Signed-off-by: Paul Bastide <[email protected]>

* Confusing error when request is targetted for an invalid tenant id #1792

1 - added layer at the rest level

Signed-off-by: Paul Bastide <[email protected]>

* Issue #1615 - Enforce configured interactions in REST layer

Signed-off-by: Mike Schroeder <[email protected]>

* Update FHIRValidationGuide.md

1. update the version references for the packaged implementation guides
2. add a section at the top to describe where to get the validation module

Signed-off-by: Lee Surprenant <[email protected]>

* Issue #1615 - address review comments

Signed-off-by: Mike Schroeder <[email protected]>

* Issue #1615 - fix whitespace

Signed-off-by: Mike Schroeder <[email protected]>

* Issue #1615 - add enum for interaction types

Signed-off-by: Mike Schroeder <[email protected]>

* Modify davinci-pdex CapabilityStatement-pdex-server.json

updated the searchRevInclude value for the Coverage resource in
pdex-server to work around https://jira.hl7.org/browse/FHIR-30338

Signed-off-by: Lee Surprenant <[email protected]>

* Issue #1494 - add Bundle.entry.search to search results

Signed-off-by: Mike Schroeder <[email protected]>

* Issue #1494 - add documentation

Signed-off-by: Mike Schroeder <[email protected]>

* Issue #1494 - address review comments

Signed-off-by: Mike Schroeder <[email protected]>

Co-authored-by: Paul Bastide <[email protected]>
Co-authored-by: Lee Surprenant <[email protected]>
Co-authored-by: Mike Schroeder <[email protected]>
Co-authored-by: Michael W Schroeder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants